Assert non-null arguments in DefaultServerRequestBuilder methods#30046
Assert non-null arguments in DefaultServerRequestBuilder methods#30046srivatsa-cfp wants to merge 6 commits intospring-projects:mainfrom
DefaultServerRequestBuilder methods#30046Conversation
srivatsa-cfp
commented
Feb 27, 2023
- 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
- 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
…er-1 fix(src): add validation for http headers and cookie
DefaultServerRequestBuilder methods
|
Note that However, we do have inconsistent state in both 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 |
|
@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? |
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
…er-Webflux Assert non-null arguments in DefaultServerRequestBuilder methods
|
Added the assert non-null checks in DefaultServerRequestBuilder in spring-webflux |
|
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. |
|
@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. |
|
@poutsma Added all the required null assertions in DefaultSertverRequestBuilder across webflux and web-mvc. |
* gh-30046: Add non-null assertions in DefaultServerRequestBuilder
|
Merged, thanks for creating a PR! |