Skip to content

aws: preserve plus sign intent inside query parameters#40619

Merged
mattklein123 merged 6 commits intoenvoyproxy:mainfrom
nbaws:query_string_encode
Aug 21, 2025
Merged

aws: preserve plus sign intent inside query parameters#40619
mattklein123 merged 6 commits intoenvoyproxy:mainfrom
nbaws:query_string_encode

Conversation

@nbaws
Copy link
Copy Markdown
Contributor

@nbaws nbaws commented Aug 7, 2025

Commit Message: aws: preserve plus sign inside query parameters

Additional Description:
Ambiguity in the way query parameters containing literal plus signs could have been encoded leads to a signature verification failure.
We now correctly preserve the intent behind plus signs when they arrive in the canonicalizer logic. A raw plus becomes a space, an encoded plus (%2B) stays as a %2B and an encoded space (%20) stays as a %20.

Risk Level: Low
Testing: Unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] #40523
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Nigel Brittain <nbaws@amazon.com>
@nbaws nbaws requested a review from mattklein123 as a code owner August 7, 2025 07:20
@nbaws nbaws marked this pull request as draft August 7, 2025 07:20
nbaws added 2 commits August 15, 2025 05:24
Signed-off-by: Nigel Brittain <nbaws@amazon.com>
Signed-off-by: Nigel Brittain <nbaws@amazon.com>
@nbaws nbaws marked this pull request as ready for review August 15, 2025 07:32
@nbaws
Copy link
Copy Markdown
Contributor Author

nbaws commented Aug 16, 2025

@niax


path_with_query =
path_with_query.substr(0, query_start + 1) + query_part + path_with_query.substr(query_end);
headers.setPath(path_with_query);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we're blindly replacing + which could be reasonably treated as a space with %2B, will it break requests that actually mean it as a space?

If this is the proper behaviour, can we link to the AWS docs or SDK that act this way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've redone this with a better attempt at preserving the intent of the original pluses, and knowing how we're treating them on the back end. It no longer touches the URL. PTAL when you have a chance.

Signed-off-by: Nigel Brittain <nbaws@amazon.com>
@nbaws nbaws marked this pull request as draft August 18, 2025 22:51
@nbaws nbaws changed the title aws: pre encode plus sign inside query parameters aws: preserve plus sign intent inside query parameters Aug 19, 2025
nbaws added 2 commits August 19, 2025 02:54
Signed-off-by: Nigel Brittain <nbaws@amazon.com>
Signed-off-by: Nigel Brittain <nbaws@amazon.com>
@nbaws nbaws marked this pull request as ready for review August 19, 2025 03:18
@nbaws nbaws requested a review from niax August 19, 2025 03:19
@mattklein123 mattklein123 merged commit 4e2078d into envoyproxy:main Aug 21, 2025
24 checks passed
melginaldi pushed a commit to melginaldi/envoy that referenced this pull request Aug 26, 2025
)

Commit Message: aws: preserve plus sign inside query parameters

Additional Description:
Ambiguity in the way query parameters containing literal plus signs
could have been encoded leads to a signature verification failure.
We now correctly preserve the intent behind plus signs when they arrive
in the canonicalizer logic. A raw plus becomes a space, an encoded plus
(%2B) stays as a %2B and an encoded space (%20) stays as a %20.

Risk Level: Low
Testing: Unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] envoyproxy#40523
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Nigel Brittain <nbaws@amazon.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
melginaldi pushed a commit to melginaldi/envoy that referenced this pull request Aug 26, 2025
)

Commit Message: aws: preserve plus sign inside query parameters

Additional Description:
Ambiguity in the way query parameters containing literal plus signs
could have been encoded leads to a signature verification failure.
We now correctly preserve the intent behind plus signs when they arrive
in the canonicalizer logic. A raw plus becomes a space, an encoded plus
(%2B) stays as a %2B and an encoded space (%20) stays as a %20.

Risk Level: Low
Testing: Unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] envoyproxy#40523
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Nigel Brittain <nbaws@amazon.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
wtzhang23 pushed a commit to wtzhang23/envoy that referenced this pull request Aug 27, 2025
)

Commit Message: aws: preserve plus sign inside query parameters

Additional Description:
Ambiguity in the way query parameters containing literal plus signs
could have been encoded leads to a signature verification failure.
We now correctly preserve the intent behind plus signs when they arrive
in the canonicalizer logic. A raw plus becomes a space, an encoded plus
(%2B) stays as a %2B and an encoded space (%20) stays as a %20.

Risk Level: Low
Testing: Unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] envoyproxy#40523
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Nigel Brittain <nbaws@amazon.com>
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.

3 participants