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

Improve guidance on basic auth in WebFlux and refine approach to deprecations [SPR-17099] #21636

Closed
spring-projects-issues opened this issue Jul 27, 2018 · 5 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 27, 2018

Juergen Zimmermann opened SPR-17099 and commented

I replaced the deprecated ExchangeFilterFunction.basicAuthentication(USERNAME, PASSWORD) with this Kotlin fragment:

ExchangeFilterFunction.ofRequestProcessor { request ->
  request.headers().setBasicAuth(USERNAME, PASSWORD)
  request.toMono()
}

 

Now I get the following stacktrace:

java.lang.UnsupportedOperationException
at java.base/java.util.Collections$UnmodifiableMap.put(Collections.java:1453)
at org.springframework.http.HttpHeaders.set(HttpHeaders.java:1512)
at org.springframework.http.HttpHeaders.setBasicAuth(HttpHeaders.java:750)
at org.springframework.http.HttpHeaders.setBasicAuth(HttpHeaders.java:720)
at de.hska.kunde.rest.KundeRestTest$beforeAll$1.apply(KundeRestTest.kt:108)
at de.hska.kunde.rest.KundeRestTest$beforeAll$1.apply(KundeRestTest.kt:93)
at org.springframework.web.reactive.function.client.ExchangeFilterFunction.lambda$ofRequestProcessor$3(ExchangeFilterFunction.java:77)
at org.springframework.web.reactive.function.client.ExchangeFilterFunction.lambda$apply$2(ExchangeFilterFunction.java:66)
at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultRequestBodyUriSpec.exchange(DefaultWebClient.java:321)
at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultRequestBodyUriSpec.retrieve(DefaultWebClient.java:367)
at de.hska.kunde.rest.KundeRestTest$Lesen.Suche mit vorhandenem Nachnamen(KundeRestTest.kt:268)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:515)
...
at java.base/java.lang.Thread.run(Thread.java:844)


Affects: 5.1 RC1

Referenced from: commits 4a18488

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 27, 2018

Juergen Hoeller commented

Looks like this is operating against a read-only HttpHeaders instance... At the very least we should provide a proper exception here, and revisit why the HttpHeaders are read-only at that point to begin with.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 6, 2018

Rossen Stoyanchev commented

The request is immutable. You need to use ClientRequest.from(request) to modify it.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2018

Juergen Zimmermann commented

OK, this replacement works, but I'd recommend to mention it in e.g. the comment for deprecation or in the release notes.

ExchangeFilterFunction.ofRequestProcessor{request ->
  ClientRequest.from(request)
      .headers{headers -> headers.setBasicAuth(username, password)}
      .build()
      .toMono()
}
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2018

Juergen Zimmermann commented

Sorry, making the code fragment as preformatted is really cumbersome...

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2018

Rossen Stoyanchev commented

I've updated ExchangeFilterFunctions.basicAuthentication(String user, String password) to delegate to HttpHeaders#setBasicAuth(user, password) internally, and I've also removed the deprecation from it since it is still useful and saves a bit of boilerplate.

I kept the no-arg method, which looks up the credentials from a request attribute, deprecated since that one is a straight-forward use of HttpHeaders when building the request:

webClient.get()
        .uri("...")
        .headers(headers -> headers.setBasicAuth(user, password))
        .exchange()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.