Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: make Headers and HeadersBuilder case-insensitive #2400

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion examples/java/hello_world/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ private void makeRequest() {
String message = "received headers with status " + status;

StringBuilder sb = new StringBuilder();
for (Map.Entry<String, List<String>> entry : responseHeaders.getHeaders().entrySet()) {
for (Map.Entry<String, List<String>> entry :
responseHeaders.caseSensitiveHeaders().entrySet()) {
String name = entry.getKey();
if (FILTERED_HEADERS.contains(name)) {
sb.append(name).append(": ").append(String.join(", ", entry.getValue())).append("\n");
Expand Down
2 changes: 1 addition & 1 deletion examples/kotlin/hello_world/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class MainActivity : Activity() {
val message = "received headers with status $status"

val sb = StringBuilder()
for ((name, value) in responseHeaders.headers) {
for ((name, value) in responseHeaders.caseSensitiveHeaders()) {
if (name in FILTERED_HEADERS) {
sb.append(name).append(": ").append(value.joinToString()).append("\n")
}
Expand Down
1 change: 1 addition & 0 deletions library/kotlin/io/envoyproxy/envoymobile/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ envoy_mobile_kt_library(
"EnvoyError.kt",
"Headers.kt",
"HeadersBuilder.kt",
"HeadersContainer.kt",
"KeyValueStore.kt",
"LogLevel.kt",
"PulseClient.kt",
Expand Down
17 changes: 8 additions & 9 deletions library/kotlin/io/envoyproxy/envoymobile/Headers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,34 @@ package io.envoyproxy.envoymobile
* To instantiate new instances, see `{Request|Response}HeadersBuilder`.
*/
open class Headers {
@Suppress("MemberNameEqualsClassName")
val headers: Map<String, List<String>>
val container: HeadersContainer

/**
* Internal constructor used by builders.
*
* @param headers: Headers to set.
jpsim marked this conversation as resolved.
Show resolved Hide resolved
jpsim marked this conversation as resolved.
Show resolved Hide resolved
*/
protected constructor(headers: Map<String, List<String>>) {
this.headers = headers
internal constructor(container: HeadersContainer) {
this.container = container
}

/**
* Get the value for the provided header name.
*
* @param name: Header name for which to get the current value.
*
* @return List<String>?, The current headers specified for the provided name.
* @return The current headers specified for the provided name.
*/
fun value(name: String): List<String>? {
return headers[name]
return container.value(name)
}

/**
* Accessor for all underlying headers as a map.
jpsim marked this conversation as resolved.
Show resolved Hide resolved
*
* @return Map<String, List<String>>, The underlying headers.
* @return The underlying headers.
*/
fun allHeaders(): Map<String, List<String>> {
return headers
fun caseSensitiveHeaders(): Map<String, List<String>> {
return container.caseSensitiveHeaders()
}
}
18 changes: 9 additions & 9 deletions library/kotlin/io/envoyproxy/envoymobile/HeadersBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ package io.envoyproxy.envoymobile
* See `{Request|Response}HeadersBuilder` for usage.
*/
open class HeadersBuilder {
protected val headers: MutableMap<String, MutableList<String>>
protected val container: HeadersContainer

/**
* Instantiate a new builder, only used by child classes.
*
* @param headers: The headers to start with.
* @param container: The headers container to start with.
*/
protected constructor(headers: MutableMap<String, MutableList<String>>) {
this.headers = headers
internal constructor(container: HeadersContainer) {
this.container = container
}

/**
Expand All @@ -28,7 +28,7 @@ open class HeadersBuilder {
if (isRestrictedHeader(name)) {
return this
}
headers.getOrPut(name) { mutableListOf<String>() }.add(value)
container.add(name, value)
return this
}

Expand All @@ -44,7 +44,7 @@ open class HeadersBuilder {
if (isRestrictedHeader(name)) {
return this
}
headers[name] = value
container.set(name, value)
return this
}

Expand All @@ -59,7 +59,7 @@ open class HeadersBuilder {
if (isRestrictedHeader(name)) {
return this
}
headers.remove(name)
container.remove(name)
return this
}

Expand All @@ -72,10 +72,10 @@ open class HeadersBuilder {
* @return HeadersBuilder, This builder.
*/
internal open fun internalSet(name: String, value: MutableList<String>): HeadersBuilder {
headers[name] = value
container.set(name, value)
return this
}

private fun isRestrictedHeader(name: String) = name.startsWith(":") ||
name.startsWith("x-envoy-mobile") || name == "host"
name.startsWith("x-envoy-mobile", true) || name.equals("host", true)
jpsim marked this conversation as resolved.
Show resolved Hide resolved
}
135 changes: 135 additions & 0 deletions library/kotlin/io/envoyproxy/envoymobile/HeadersContainer.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package io.envoyproxy.envoymobile

/*
* The container that manages the underlying headers map.
* It maintains the original casing of passed header names.
* It treats headers names as case-insensitive for the purpose
* of header lookups and header name conflict resolutions.
*/
open class HeadersContainer {
protected val headers: MutableMap<String, Header>

/*
* Represents a header name together with all of its values.
* It preserves the original casing of the header name.
*/
data class Header(val name: String, var value: MutableList<String>) {
constructor(name: String) : this(name, mutableListOf())

fun add(value: List<String>) {
this.value.addAll(value)
}

fun add(value: String) {
this.value.add(value)
}
}

/*
* Instantiate a new instance of the receiver using the provided headers map
*
* @param headers: The headers to start with.
*/
internal constructor(headers: Map<String, MutableList<String>>) {
var underlyingHeaders = mutableMapOf<String, Header>()
/*
* Dictionaries are unordered collections. Process headers with names
* that are the same when lowercased in an alphabetical order to avoid a situation
* in which the result of the initialization is non-derministic i.e., we want
* mapOf("A" to listOf("1"), "a" to listOf("2")) headers to be always converted to
* mapOf("A" to listOf("1", "2")) and never to mapOf("a" to listOf("2", "1")).
*
* If a given header name already exists in the processed headers map, check
* if the currently processed header name is before the existing header name as
* determined by an alphabetical order.
*/
headers.forEach {
val lowercased = it.key.lowercase()
val existing = underlyingHeaders[lowercased]

if (existing == null) {
underlyingHeaders[lowercased] = Header(it.key, it.value)
} else if (existing.name > it.key) {
underlyingHeaders[lowercased] = Header(it.key, (it.value + existing.value).toMutableList())
} else {
underlyingHeaders[lowercased]?.add(it.value)
}
}

this.headers = underlyingHeaders
}

companion object {
/*
* Create a new instance of the receiver using a provider headers map.
* Not implemented as a constructor due to conflicting JVM signatures with
* other constructors.
*
* @param headers: The headers to create the container with.
*/
fun create(headers: Map<String, List<String>>) : HeadersContainer {
return HeadersContainer(headers.mapValues { it.value.toMutableList() })
}
}

/*
* Add a value to a header with a given name.
*
* @param name The name of the header. For the purpose of headers lookup
* and header name conflict resolution, the name of the header
* is considered to be case-insensitive.
* @param value The value to add.
*/
fun add(name: String, value: String) {
val lowercased = name.lowercase()
headers[lowercased]?.let { it.add(value) } ?: run {
headers.put(lowercased, Header(name, mutableListOf(value)))
}
}


/*
* Set the value of a given header.
*
* @param name The name of the header.
* @param value The value to set the header value to.
*/
fun set(name: String, value: List<String>) {
headers[name.lowercase()] = Header(name, value.toMutableList())
}

/*
* Remove a given header.
*
* @param name The name of the header to remove.
*/
fun remove(name: String) {
headers.remove(name)
jpsim marked this conversation as resolved.
Show resolved Hide resolved
}

/*
* Get the value for the provided header name.
*
* @param name The case-insensitive header name for which to
* get the current value.
* @return The value associated with a given header.
*/
fun value(name: String): List<String>? {
return headers[name.lowercase()]?.value
}

/*
* Accessor for all underlying case-sensitive headers. When possible,
* use case-insensitive accessors instead.
*
* @return The underlying headers.
*/
fun caseSensitiveHeaders(): Map<String, List<String>> {
var caseSensitiveHeaders = mutableMapOf<String, List<String>>()
headers.forEach {
caseSensitiveHeaders.put(it.value.name, it.value.value)
}

return caseSensitiveHeaders
}
}
10 changes: 4 additions & 6 deletions library/kotlin/io/envoyproxy/envoymobile/RequestHeaders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ open class RequestHeaders : Headers {
*
* @param headers: Headers to set.
*/
internal constructor(headers: Map<String, List<String>>) : super(headers)
internal constructor(headers: Map<String, List<String>>) : super(HeadersContainer.create(headers))

internal constructor(container: HeadersContainer): super(container)

/**
* Method for the request.
Expand Down Expand Up @@ -49,9 +51,5 @@ open class RequestHeaders : Headers {
*
* @return RequestHeadersBuilder, The new builder.
*/
fun toRequestHeadersBuilder() = RequestHeadersBuilder(
headers.mapValues {
it.value.toMutableList()
}.toMutableMap()
)
fun toRequestHeadersBuilder() = RequestHeadersBuilder(container)
}
16 changes: 12 additions & 4 deletions library/kotlin/io/envoyproxy/envoymobile/RequestHeadersBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,30 @@ class RequestHeadersBuilder : HeadersBuilder {
authority: String,
path: String
) :
super(
mutableMapOf(
super(HeadersContainer(
mapOf(
":authority" to mutableListOf(authority),
":method" to mutableListOf(method.stringValue),
":path" to mutableListOf(path),
":scheme" to mutableListOf(scheme)
)
)
)

/**
* Instantiate a new builder. Used only by RequestHeaders to convert back to
* RequestHeadersBuilder.
*
* @param headers: The headers to start with.
*/
internal constructor(headers: MutableMap<String, MutableList<String>>) : super(headers)
internal constructor(headers: Map<String, MutableList<String>>) : super(HeadersContainer(headers))

/**
* Instantiate a new builder.
*
* @param container: The headers container to start with.
*/
internal constructor(container: HeadersContainer) : super(container)

override fun add(name: String, value: String): RequestHeadersBuilder {
super.add(name, value)
Expand Down Expand Up @@ -92,6 +100,6 @@ class RequestHeadersBuilder : HeadersBuilder {
* @return RequestHeaders, New instance of request headers.
*/
fun build(): RequestHeaders {
return RequestHeaders(headers)
return RequestHeaders(container)
}
}
13 changes: 8 additions & 5 deletions library/kotlin/io/envoyproxy/envoymobile/RequestTrailers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ class RequestTrailers : Trailers {
*/
internal constructor(trailers: Map<String, List<String>>) : super(trailers)

/**
* Instantiate a new builder.
*
* @param container: The headers container to start with.
*/
internal constructor(container: HeadersContainer) : super(container)

/**
* Convert the trailers back to a builder for mutation.
*
* @return RequestTrailersBuilder, The new builder.
*/
fun toRequestTrailersBuilder() = RequestTrailersBuilder(
headers.mapValues {
it.value.toMutableList()
}.toMutableMap()
)
fun toRequestTrailersBuilder() = RequestTrailersBuilder(container)
}
20 changes: 14 additions & 6 deletions library/kotlin/io/envoyproxy/envoymobile/RequestTrailersBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,26 @@ package io.envoyproxy.envoymobile
* Builder used for constructing instances of `RequestTrailers`.
*/
class RequestTrailersBuilder : HeadersBuilder {
/**
* Initialize a new instance of the builder.
/*
* Instantiate a new builder.
*/
constructor() : super(mutableMapOf())
internal constructor() : super(HeadersContainer(mapOf()))

/**
/*
* Instantiate a new instance of the builder.
*
* @param container: The headers container to start with.
*/
internal constructor(container: HeadersContainer) : super(container)

/*
* Instantiate a new builder. Used only by RequestTrailers to convert back to
* RequestTrailersBuilder.
*
* @param trailers: The trailers to start with.
*/
internal constructor(trailers: MutableMap<String, MutableList<String>>) : super(trailers)
internal constructor(trailers: MutableMap<String, MutableList<String>>)
: super(HeadersContainer(trailers))

override fun add(name: String, value: String): RequestTrailersBuilder {
super.add(name, value)
Expand Down Expand Up @@ -43,6 +51,6 @@ class RequestTrailersBuilder : HeadersBuilder {
* @return RequestTrailers, New instance of request trailers.
*/
fun build(): RequestTrailers {
return RequestTrailers(headers)
return RequestTrailers(container)
}
}
Loading