Skip to content

Commit

Permalink
api: disallow setting 'host' header directly (#2275)
Browse files Browse the repository at this point in the history
Description: The ':authority' header is used to direct the connection internally and this is set when initializing the builder.

Fixes #2244.

Risk Level: Low
Testing: Unit

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
  • Loading branch information
goaway authored and jpsim committed Nov 29, 2022
1 parent 5812e4f commit abf2d0b
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 2 deletions.
1 change: 1 addition & 0 deletions mobile/docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Pending Release
Breaking changes:

- api: replace the `drainConnections()` method with a broader `resetConnectivityState()`. (:issue:`#2225 <2225>`).
- api: disallow setting 'host' header directly (:issue:`#2275 <2275>`)
- net: enable happy eyeballs by default (:issue:`#2272 <2272>`)

Bugfixes:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,5 @@ open class HeadersBuilder {
}

private fun isRestrictedHeader(name: String) = name.startsWith(":") ||
name.startsWith("x-envoy-mobile")
name.startsWith("x-envoy-mobile") || name == "host"
}
2 changes: 1 addition & 1 deletion mobile/library/swift/HeadersBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Foundation
private let kRestrictedPrefixes = [":", "x-envoy-mobile"]

private func isRestrictedHeader(name: String) -> Bool {
return kRestrictedPrefixes.contains { name.hasPrefix($0) }
return name == "host" || kRestrictedPrefixes.contains { name.hasPrefix($0) }
}

/// Base builder class used to construct `Headers` instances.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,12 @@ class RequestHeadersBuilderTest {
)
.add(":x-foo", "123")
.add("x-envoy-mobile-foo", "abc")
.add("host", "example.com")
.build()

assertThat(headers.allHeaders()).doesNotContainKey(":x-foo")
assertThat(headers.allHeaders()).doesNotContainKey("x-envoy-mobile-foo")
assertThat(headers.allHeaders()).doesNotContainKey("host")
}

@Test
Expand Down
15 changes: 15 additions & 0 deletions mobile/test/swift/HeadersBuilderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ final class HeadersBuilderTests: XCTestCase {
XCTAssertEqual(["x-foo": ["abc"]], headers)
}

func testRestrictedHeadersAreNotSettable() {
let headers = RequestHeadersBuilder(method: .get, authority: "example.com", path: "/")
.add(name: "host", value: "example.com")
.set(name: ":scheme", value: ["http"])
.set(name: ":path", value: ["/nope"])
.headers
let expected = [
":authority": ["example.com"],
":path": ["/"],
":method": ["GET"],
":scheme": ["https"],
]
XCTAssertEqual(expected, headers)
}

func testBuildersAreEqualIfUnderlyingHeadersAreEqual() {
let builder1 = RequestHeadersBuilder(headers: ["x-foo": ["123"], "x-bar": ["abc"]])
let builder2 = RequestHeadersBuilder(headers: ["x-foo": ["123"], "x-bar": ["abc"]])
Expand Down

0 comments on commit abf2d0b

Please sign in to comment.