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

Stop double-encoding path in various filter functions #3437

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rworsnop
Copy link

Fixes gh-3436

@rworsnop rworsnop marked this pull request as ready for review June 25, 2024 16:17
@rworsnop rworsnop changed the title Stop double-encoding path in stripPrefix Stop double-encoding path in various filter functions Jun 25, 2024
@Sroca3
Copy link

Sroca3 commented Jun 25, 2024

@spencergibb does closing the associated issue mean this is going to get merged at some point?

@spencergibb
Copy link
Member

No, just if you're going to submit a PR immediately after opening an issue, the issue is unnecessary.

@dvag-yannick-reifschneider

I have encountered this issue as well. As a workaround I implemented a custom filter function, which reverses the url decoding. You have to add this filter after StripPrefix, RewritePath or other affected filters.

public class UrlDecodeFilter {
    public static HandlerFilterFunction<ServerResponse, ServerResponse> urlDecode() {
        return HandlerFilterFunction.ofRequestProcessor((ServerRequest request) -> {
            String urlDecodedUri = UriUtils.decode(request.uri().toString(), StandardCharsets.UTF_8);
            return ServerRequest.from(request).uri(URI.create(urlDecodedUri)).build();
        });
    }
}

@jensmatw
Copy link

I haven't seen this and opened another PR for rewritePath. i like the different approach, using build(true) to tell the builder that it is already encoded but maybe there is something to consider, at least for rewritePath there is a difference in the behaviour between our different solutions:
if we replace getRawPath() with getPath() to get the path initially to apply the user created regex, it is applied to the decoded string, if we use build(true) in the end to prevent double encoding the regex is applied on the encoded String

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stripPrefix is double-encoding paths
6 participants