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

fix: Ensure protocol slashes are preserved in getSanitizedPath #33978

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sanhee
Copy link

@sanhee sanhee commented Nov 27, 2024

Description

When using WebClient with UriBuilder to construct URIs, I encountered a problem where the protocol's double slashes (://) were incorrectly reduced to a single slash (:/).
As a result, malformed URIs such as http:/jsonplaceholder.typicode.com/users were generated, causing WebClient to throw a Host is not specified exception.
This prevented the request from reaching the server and resulted in a 500 error.

This behavior occurred because the getSanitizedPath method in the UriComponentsBuilder class attempted to remove redundant slashes, but inadvertently changed the protocol structure by failing to preserve the :// delimiter.

Error Sample

org.springframework.web.reactive.function.client.WebClientRequestException: Host is not specified
	at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.lambda$wrapException$9(ExchangeFunctions.java:137) ~[spring-webflux-6.2.0.jar:6.2.0]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Assembly trace from producer [reactor.core.publisher.MonoErrorSupplied] :
	reactor.core.publisher.Mono.error(Mono.java:315)
	org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.wrapException(ExchangeFunctions.java:137)
Error has been observed at the following site(s):
	*____________Mono.error ⇢ at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.wrapException(ExchangeFunctions.java:137)
	*____Mono.onErrorResume ⇢ at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.exchange(ExchangeFunctions.java:106)
	|_             Mono.map ⇢ at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.exchange(ExchangeFunctions.java:107)
	|_        Mono.doOnNext ⇢ at org.springframework.web.reactive.function.client.DefaultWebClient$ObservationFilterFunction.filter(DefaultWebClient.java:745)
	*____________Mono.defer ⇢ at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultRequestBodyUriSpec.lambda$exchange$12(DefaultWebClient.java:467)
	|_           checkpoint ⇢ Request to GET http:/jsonplaceholder.typicode.com/users [DefaultWebClient]
	|_   Mono.switchIfEmpty ⇢ at 
...

Changes

  • Refined logic to ensure protocol slashes are preserved.
  • Cleaned up redundant slashes in the path without affecting the protocol.

Test

  • To validate and fix this behavior, I tested the private getSanitizedPath method using reflection.
    • Since the method is private and part of an internal class, reflection was necessary to access and evaluate its behavior in isolation.
  • Verified with multiple inputs, including paths with and without protocols, to ensure correct behavior.
AS-IS (Existing Issue) TO-BE (Fixed Behavior)
스크린샷 2024-11-27 오후 11 50 32 image

Example Test Case

Input: http://example.com//path/to//resource
Output: http://example.com/path/to/resource

Input: https://example.com//path
Output: https://example.com/path

Input: example.com//path
Output: example.com/path

Fixed an issue where `getSanitizedPath` incorrectly removed slashes in protocols like `http://`.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 27, 2024
@sanhee
Copy link
Author

sanhee commented Dec 22, 2024

Hi @bclozel
Hope you're well! About a month ago, I noticed that the first build for this PR failed, but I've since fixed the issue and pushed an update.
It seems that the actions need to be retriggered for the updated changes.

If you have some time, would you mind retriggering the actions?
I'd also really appreciate it if you could take a look at the changes and let me know if there's anything else I should adjust or clarify! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants