-
Notifications
You must be signed in to change notification settings - Fork 38k
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
Avoid silent default to http://localhost:80 in ReactorHttpClientConnector [SPR-15782] #20337
Comments
Rossen Stoyanchev commented This is expected behavior actually since at the time of parsing, the URI template looks like a path. Depending on how much you need to customize (i.e. literal vs variable): WebClient.builder()
.build()
.get()
.uri("http://{host}:{port}/api/v4/groups/{groupName}", "localhost", "8080", "foo") You could also set the baseUrl at the builder level: WebClient.builder()
.baseUrl("http://{host}:{port}")
.build()
.get()
.uri("/api/v4/groups/{groupName}", "localhost", "8080", "foo"); |
Abhijit Sarkar commented
|
Rossen Stoyanchev commented Abhijit Sarkar point taken about the confusion. I'm trying to see what we can do to improve. First, renaming from Logging a warning if a URI variable has a protocol won't either do because it is legitimate to insert a URI, email address or anything into a path segment (e.g. as an identifier for some resource) in which case the URI variable value is encoded so that URI reserved characters won't change the URI structure. Taking a closer look at what actually happens. The request that the WebClient prepares based on your example does not have scheme/host/port. So it is the underlying client (Reactor Netty) that defaults to http://localhost:8080. That is a clear issue in my opinion and the behavior could also vary when other HTTP clients are supported. Given that |
Rossen Stoyanchev commented Setting to RC3 since we'll make an improvement either way. |
Abhijit Sarkar commented I think throwing an exception makes sense. That should've been among the options I'd suggested. |
Abhijit Sarkar opened SPR-15782 and commented
In the following code,
baseUrl
is not replaced and defaults tohttp://localhost:80
.Affects: 5.0 RC2
Referenced from: commits 147368e
The text was updated successfully, but these errors were encountered: