Skip to content

Conversation

@Laonel
Copy link
Member

@Laonel Laonel commented Dec 3, 2025

Fixes request rewriting when query parameters contain a value that matches a percent-encoded value (e.g. %2F).

Previously, such values were not encoded when adding query parameters to the rewritten request, as the method used internally by JerseyUriBuilder.queryParam to encode values (UriComponent.contextualEncode) is recognizing percent-encoded values to avoid double-encoding. This led to changing the original request, and subsequently the remote denying the request with SignatureDoesNotMatch error.

To fix this, we can encode all query parameter values before passing them to the URI builder, as we know that they require encoding.

@cla-bot cla-bot bot added the cla-signed label Dec 3, 2025
@Laonel Laonel marked this pull request as draft December 3, 2025 12:56
@Laonel Laonel force-pushed the patryk/target-query-encoding branch from 0be84e6 to 3c63818 Compare December 3, 2025 13:13
Fixes request rewriting when query parameters contain a value that
matches a percent-encoded value (e.g. `%2F`).

Previously, such values were not encoded when adding query parameters
to the rewritten request, as the method used internally by
`JerseyUriBuilder.queryParam` to encode values
(UriComponent.contextualEncode) is recognizing percent-encoded values
to avoid double-encoding. This led to changing the original
request, and subsequently the remote denying the request with
`SignatureDoesNotMatch` error.

To fix this, we can encode all query parameter values before passing
them to the URI builder, as we know that they require encoding.
@Laonel Laonel force-pushed the patryk/target-query-encoding branch from 3c63818 to dbfe71e Compare December 3, 2025 13:24
@Laonel Laonel marked this pull request as ready for review December 3, 2025 13:26
Copy link
Member

@vagaerg vagaerg left a comment

Choose a reason for hiding this comment

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

LGTM - nice catch, as we decode them when building a request

@vagaerg vagaerg merged commit 9210930 into trinodb:main Dec 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants