Skip to content

Commit

Permalink
Ensure user-provieded metadata has lowercase keys
Browse files Browse the repository at this point in the history
Motivation:

HTTP/2 requires header field names to be lowercased. Metadata keys
should be lowercased automatically.

Modifications:

Lowercase user-provided metadata keys.

Results:

Fewer bugs
  • Loading branch information
glbrntt committed Nov 20, 2024
1 parent 565aae3 commit b5c4d94
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 8 deletions.
9 changes: 6 additions & 3 deletions Sources/GRPCNIOTransportCore/GRPCStreamStateMachine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,8 @@ extension GRPCStreamStateMachine {
}

for metadataPair in customMetadata {
headers.add(name: metadataPair.key, value: metadataPair.value.encoded())
// Lowercase the field names for user-provided metadata.
headers.add(name: metadataPair.key.lowercased(), value: metadataPair.value.encoded())
}

return headers
Expand Down Expand Up @@ -1248,7 +1249,8 @@ extension GRPCStreamStateMachine {
}

for metadataPair in customMetadata {
headers.add(name: metadataPair.key, value: metadataPair.value.encoded())
// Lowercase the field names for user-provided metadata.
headers.add(name: metadataPair.key.lowercased(), value: metadataPair.value.encoded())
}
}

Expand Down Expand Up @@ -1827,7 +1829,8 @@ extension HPACKHeaders {
}

for (key, value) in metadata {
trailers.add(name: key, value: value.encoded())
// Lowercase the field names for user-provided metadata.
trailers.add(name: key.lowercased(), value: value.encoded())
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions Tests/GRPCNIOTransportHTTP2Tests/ControlMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ struct ControlInput: Codable {
/// removed (":path" should become "echo-path").
var echoMetadataInTrailers: Bool = false

/// Key-value pairs to add to the initial metadata.
var initialMetadataToAdd: [String: String] = [:]

/// Key-value pairs to add to the trailing metadata.
var trailingMetadataToAdd: [String: String] = [:]

struct Status: Codable {
var code: GRPCCore.Status.Code
var message: String
Expand Down
22 changes: 17 additions & 5 deletions Tests/GRPCNIOTransportHTTP2Tests/ControlService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ extension ControlService {

// Check if the request is for a trailers-only response.
if let status = message.status, message.isTrailersOnly {
let trailers = message.echoMetadataInTrailers ? request.metadata.echo() : [:]
var trailers = message.echoMetadataInTrailers ? request.metadata.echo() : [:]
for (key, value) in message.trailingMetadataToAdd {
trailers.addString(value, forKey: key)
}
let code = Status.Code(rawValue: status.code.rawValue).flatMap { RPCError.Code($0) }

if let code = code {
Expand All @@ -117,7 +120,10 @@ extension ControlService {
}

// Not a trailers-only response. Should the metadata be echo'd back?
let metadata = message.echoMetadataInHeaders ? request.metadata.echo() : [:]
var metadata = message.echoMetadataInHeaders ? request.metadata.echo() : [:]
for (key, value) in message.initialMetadataToAdd {
metadata.addString(value, forKey: key)
}

// The iterator needs to be transferred into the response. This is okay: we won't touch the
// iterator again from the current concurrency domain.
Expand Down Expand Up @@ -174,10 +180,13 @@ extension ControlService {

// Check whether the RPC should be finished (i.e. the input `hasStatus`).
guard let status = input.status else {
if input.echoMetadataInTrailers {
if input.echoMetadataInTrailers || !input.trailingMetadataToAdd.isEmpty {
// There was no status in the input, but echo metadata in trailers was set. This is an
// implicit 'ok' status.
let trailers = input.echoMetadataInTrailers ? metadata.echo() : [:]
var trailers = input.echoMetadataInTrailers ? metadata.echo() : [:]
for (key, value) in input.trailingMetadataToAdd {
trailers.addString(value, forKey: key)
}
return .return(trailers)
} else {
// No status, and not echoing back metadata. Continue consuming the input stream.
Expand All @@ -186,7 +195,10 @@ extension ControlService {
}

// Build the trailers.
let trailers = input.echoMetadataInTrailers ? metadata.echo() : [:]
var trailers = input.echoMetadataInTrailers ? metadata.echo() : [:]
for (key, value) in input.trailingMetadataToAdd {
trailers.addString(value, forKey: key)
}

if status.code == .ok {
return .return(trailers)
Expand Down
32 changes: 32 additions & 0 deletions Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,38 @@ final class HTTP2TransportTests: XCTestCase {
}
}
}

func testUppercaseClientMetadataKey() async throws {
try await self.forEachTransportPair { control, _, _ in
let request = ClientRequest<ControlInput>(
message: .with {
$0.echoMetadataInHeaders = true
$0.numberOfMessages = 1
},
metadata: ["UPPERCASE-KEY": "value"]
)
try await control.unary(request: request) { response in
// Keys will be lowercase before being sent over the wire.
XCTAssertEqual(Array(response.metadata["echo-uppercase-key"]), ["value"])
}
}
}

func testUppercaseServerMetadataKey() async throws {
try await self.forEachTransportPair { control, _, _ in
let request = ClientRequest<ControlInput>(
message: .with {
$0.initialMetadataToAdd["UPPERCASE-KEY"] = "initial"
$0.trailingMetadataToAdd["UPPERCASE-KEY"] = "trailing"
$0.numberOfMessages = 1
}
)
try await control.unary(request: request) { response in
XCTAssertEqual(Array(response.metadata["uppercase-key"]), ["initial"])
XCTAssertEqual(Array(response.trailingMetadata["uppercase-key"]), ["trailing"])
}
}
}
}

extension [HTTP2TransportTests.Transport] {
Expand Down

0 comments on commit b5c4d94

Please sign in to comment.