- Add Null check for Http Header Name , https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/HttpHeaders.html#add(java.lang.String,java.lang.String). As per doc only header values can be nullable
- Add Null check for Http HeaderConsumer
- Add Null check for Cookie Name
- Add Null check for CookierConsumer
Comment From: sbrannen
Note that @NonNullApi is specified at the package level in package-info.java. Thus, none of those arguments are allowed to be null.
However, we do have inconsistent state in both DefaultServerRequestBuilder variants with regard to explicit non-null assertions.
If we introduce additional non-null assertions for those arguments, we would want to do that consistently for all such methods and consistently across both DefaultServerRequestBuilder variants (MVC and WebFlux).
Comment From: srivatsa-cfp
@sbrannen Yes specification says it to be not null, but no implementation might cause an inconsistent. I will verify it to be consistent across MVC & WebFlux. Do you any further concerns on non-null assertions?
Comment From: srivatsa-cfp
Added the assert non-null checks in DefaultServerRequestBuilder in spring-webflux
Comment From: sbrannen
@srivatsa-cfp,
We may decide to introduce non-null assertions in those builder methods, or we may decide to remove all of the non-null assertions and to rely on null-safety annotations instead.
So, please hold off on making further changes until the team has reached a decision on this topic.
Comment From: poutsma
@srivatsa-cfp We have decided that null assertions would be useful to have in builder methods, so please continue with your PR and let me know when it's done.
Comment From: srivatsa-cfp
@poutsma Added all the required null assertions in DefaultSertverRequestBuilder across webflux and web-mvc.
Comment From: poutsma
Merged, thanks for creating a PR!