NPE in ClientRequest.attribute(name) when accessing null value [SPR-17486]

[spring-projects-issues]

João Eiras opened SPR-17486 and commented

Hi.

I have the following code

webClient.get()
  .uri(b -> b.path("/endpoint").build())
  .attribute("x", null)
  .retrieve()
  .bodyToMono(String.class)
  .block();

I also have a exchangeFilter registered in the webClient which looks up attribute "x". When calling clientRequest.attribute("x") then I get a NPE.

java.lang.NullPointerException: null
  at java.util.Objects.requireNonNull(Objects.java:203)
  at java.util.Optional.<init>(Optional.java:96)
  at java.util.Optional.of(Optional.java:108)
  at org.springframework.web.reactive.function.client.ClientRequest.attribute(ClientRequest.java:81)

The solution in https://github.com/spring-projects/spring-framework/blob/master/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java#L88

default Optional<Object> attribute(String name) {
    return Optional.ofNullable(attributes().get(name));
}

Thank you.

---

Affects: 5.0.10, 5.1 GA

Referenced from: commits 5382260, 818c2aa, d96a7b4

Backported to: 5.0.11

[spring-projects-issues]

Juergen Hoeller commented

Technically, we don't really intend null values to be stored there, as indicated by the missing @Nullable declaration in attribute(name, value) in the builder. That said, if it happens to be stored that way, we should leniently handle it.

[spring-projects-issues]

João Eiras commented

as indicated by the missing @Nullable

Well, given we write typically a call to webClient.method(...) with many chained calls to the methods that build the request, it would be very annoying to break the chain to do a null check of some variable I might want to pass as an attribute.
Also, passing a null value could be a way to remove a previously set attribute.

So, I think it's best for the RequestHeadersSpec.attribute(name, value) API to be flexible.

Thanks for the fix though :)

[spring-projects-issues]

João Eiras commented

Oh, another thing to remark is that there is the API

webClient.get()
  .uri(...)
  .atributes(map -> {
    map.put("Weird stuff", null");
  })
  .retrieve()
  ...;

So whatever happens in the attributes() callback you have little control over.