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

Conversation

Augustyniak
Copy link
Contributor

@Augustyniak Augustyniak commented Jul 1, 2022

Description: An Android change that mimics iOS changes from #2383. Make the lookup of headers in HeadersBuilder and Headers case-insensitive and preserve the original casing of headers.
Risk Level: Low
Testing: Unit
Docs Changes: Done
Release Notes: Done
Fixes: #2390

Signed-off-by: Rafal Augustyniak [email protected]

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
@Augustyniak
Copy link
Contributor Author

Blocked by #2406

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Augustyniak added a commit that referenced this pull request Jul 5, 2022
Description: Update codespell to v2.1.0 due to an issue that surfaced in #2400 : `/usr/local/Cellar/[email protected]/3.10.5/Frameworks/Python.framework/Versions/3.10/lib/python3.10/codecs.py:905: RuntimeWarning: line buffering (buffering=1) isn't supported in binary mode, the default buffer size will be used
  file = builtins.open(filename, mode, buffering)`. According to codespell-project/codespell#1331 the update is what fixes it.
Risk Level: Low, linting change only
Testing: N/A
Docs Changes: N/A
Release Notes: Done

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
@Augustyniak Augustyniak marked this pull request as ready for review July 5, 2022 15:50
@Augustyniak Augustyniak requested review from goaway and jpsim July 5, 2022 15:51
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
jpsim
jpsim previously approved these changes Jul 5, 2022
Copy link
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 with suggestions

library/kotlin/io/envoyproxy/envoymobile/Headers.kt Outdated Show resolved Hide resolved
library/kotlin/io/envoyproxy/envoymobile/Headers.kt Outdated Show resolved Hide resolved
library/kotlin/io/envoyproxy/envoymobile/HeadersBuilder.kt Outdated Show resolved Hide resolved
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: Rafal Augustyniak <[email protected]>
@Augustyniak Augustyniak enabled auto-merge (squash) July 6, 2022 02:24
@Augustyniak Augustyniak requested a review from jpsim July 6, 2022 12:46
return this
}

private fun isRestrictedHeader(name: String) = name.startsWith(":") ||
name.startsWith("x-envoy-mobile") || name == "host"
name.startsWith("x-envoy-mobile", ignoreCase = true) || name.equals("host", ignoreCase = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, the parameter label helps a lot here.

Copy link
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nice work! Can you update the version_history.rst entry to reflect that this is now done for Kotlin and Swift APIs alike?

@Augustyniak Augustyniak merged commit 52d6a4f into main Jul 6, 2022
@Augustyniak Augustyniak deleted the andr-make-headers-and-headersbuilder-case-insensitive branch July 6, 2022 14:27
Augustyniak added a commit that referenced this pull request Jul 6, 2022
Description: Updating version history to reflect user facing changes introduced in #2400.
Risk Level: None
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <[email protected]>
Augustyniak added a commit that referenced this pull request Jul 7, 2022
…ors public (#2410)

Description: Revert constructors' visibility to the state from before #2400. These constructors were supposed to be public.
Risk Level: None
Testing: None
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <[email protected]>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 29, 2022
Description: Update codespell to v2.1.0 due to an issue that surfaced in envoyproxy/envoy-mobile#2400 : `/usr/local/Cellar/[email protected]/3.10.5/Frameworks/Python.framework/Versions/3.10/lib/python3.10/codecs.py:905: RuntimeWarning: line buffering (buffering=1) isn't supported in binary mode, the default buffer size will be used
  file = builtins.open(filename, mode, buffering)`. According to codespell-project/codespell#1331 the update is what fixes it.
Risk Level: Low, linting change only
Testing: N/A
Docs Changes: N/A
Release Notes: Done

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: JP Simard <[email protected]>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 29, 2022
Description: Updating version history to reflect user facing changes introduced in envoyproxy/envoy-mobile#2400.
Risk Level: None
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: JP Simard <[email protected]>
jpsim pushed a commit to envoyproxy/envoy that referenced this pull request Nov 29, 2022
…ors public (#2410)

Description: Revert constructors' visibility to the state from before envoyproxy/envoy-mobile#2400. These constructors were supposed to be public.
Risk Level: None
Testing: None
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <[email protected]>
Signed-off-by: JP Simard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

android: make Headers and HeadersBuilder case-insensitive
2 participants