From e52ee01ec8eac712808b1de8aba9670876c7e8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Mon, 25 Mar 2024 11:10:30 +0100 Subject: [PATCH] Perform NullAway build-time checks in spring-web Also in spring-websocket. See gh-32475 --- gradle/spring-module.gradle | 2 +- .../main/java/org/springframework/util/CollectionUtils.java | 1 + .../src/main/java/org/springframework/util/ObjectUtils.java | 1 + .../web/client/DefaultResponseErrorHandler.java | 1 + .../org/springframework/web/client/DefaultRestClient.java | 1 + .../context/request/RequestAttributesThreadLocalAccessor.java | 3 +++ .../context/request/async/StandardServletAsyncWebRequest.java | 3 +++ .../web/context/request/async/WebAsyncManager.java | 2 ++ .../support/ServletContextResourcePatternResolver.java | 1 + .../java/org/springframework/web/cors/CorsConfiguration.java | 2 ++ .../java/org/springframework/web/method/HandlerMethod.java | 2 +- .../support/AbstractMultipartHttpServletRequest.java | 1 + .../support/StandardMultipartHttpServletRequest.java | 2 ++ .../web/server/adapter/HttpWebHandlerAdapter.java | 1 + .../web/server/session/InMemoryWebSessionStore.java | 4 ++++ .../web/service/invoker/HttpServiceMethod.java | 1 + .../web/service/invoker/HttpServiceProxyFactory.java | 2 +- 17 files changed, 27 insertions(+), 3 deletions(-) diff --git a/gradle/spring-module.gradle b/gradle/spring-module.gradle index f571ecd0e2a5..2b5c46377032 100644 --- a/gradle/spring-module.gradle +++ b/gradle/spring-module.gradle @@ -118,7 +118,7 @@ tasks.withType(JavaCompile).configureEach { disableAllChecks = true option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract") option("NullAway:AnnotatedPackages", "org.springframework.core,org.springframework.expression," + - "org.springframework.web.reactive,org.springframework.web.servlet") + "org.springframework.web") option("NullAway:UnannotatedSubPackages", "org.springframework.instrument,org.springframework.context.index," + "org.springframework.asm,org.springframework.cglib,org.springframework.objenesis," + "org.springframework.javapoet,org.springframework.aot.nativex.substitution") diff --git a/spring-core/src/main/java/org/springframework/util/CollectionUtils.java b/spring-core/src/main/java/org/springframework/util/CollectionUtils.java index 17bf57391467..efc2d087c14f 100644 --- a/spring-core/src/main/java/org/springframework/util/CollectionUtils.java +++ b/spring-core/src/main/java/org/springframework/util/CollectionUtils.java @@ -62,6 +62,7 @@ public abstract class CollectionUtils { * @param collection the Collection to check * @return whether the given Collection is empty */ + @Contract("null -> true") public static boolean isEmpty(@Nullable Collection collection) { return (collection == null || collection.isEmpty()); } diff --git a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java index c807e0b6cfad..70f6f9f64ce7 100644 --- a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java @@ -101,6 +101,7 @@ public static boolean isCompatibleWithThrowsClause(Throwable ex, @Nullable Class * either an Object array or a primitive array. * @param obj the object to check */ + @Contract("null -> false") public static boolean isArray(@Nullable Object obj) { return (obj != null && obj.getClass().isArray()); } diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java index 86589daaac01..bd1fc19e4f13 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java @@ -201,6 +201,7 @@ else if (statusCode.is5xxServerError()) { * {@link RestClientResponseException#setBodyConvertFunction(Function)}. * @since 6.0 */ + @SuppressWarnings("NullAway") protected Function initBodyConvertFunction(ClientHttpResponse response, byte[] body) { Assert.state(!CollectionUtils.isEmpty(this.messageConverters), "Expected message converters"); return resolvableType -> { diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java index 62400f6606f1..ca71946724a1 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java @@ -522,6 +522,7 @@ else if (CollectionUtils.isEmpty(defaultHeaders)) { } } + @SuppressWarnings("NullAway") private ClientHttpRequest createRequest(URI uri) throws IOException { ClientHttpRequestFactory factory; if (DefaultRestClient.this.interceptors != null) { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessor.java b/spring-web/src/main/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessor.java index a3e74cb87910..136127bc6754 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessor.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessor.java @@ -18,6 +18,8 @@ import io.micrometer.context.ThreadLocalAccessor; +import org.springframework.lang.Nullable; + /** * Adapt {@link RequestContextHolder} to the {@link ThreadLocalAccessor} contract * to assist the Micrometer Context Propagation library with @@ -40,6 +42,7 @@ public Object key() { } @Override + @Nullable public RequestAttributes getValue() { return RequestContextHolder.getRequestAttributes(); } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java index b8f767c57725..6c3f3d5f3bbf 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java @@ -84,6 +84,7 @@ public StandardServletAsyncWebRequest(HttpServletRequest request, HttpServletRes * @param previousRequest the existing request from the last dispatch * @since 5.3.33 */ + @SuppressWarnings("NullAway") StandardServletAsyncWebRequest(HttpServletRequest request, HttpServletResponse response, @Nullable StandardServletAsyncWebRequest previousRequest) { @@ -243,6 +244,7 @@ public void setAsyncWebRequest(StandardServletAsyncWebRequest asyncWebRequest) { } @Override + @SuppressWarnings("NullAway") public ServletOutputStream getOutputStream() throws IOException { int level = obtainLockAndCheckState(); try { @@ -262,6 +264,7 @@ public ServletOutputStream getOutputStream() throws IOException { } @Override + @SuppressWarnings("NullAway") public PrintWriter getWriter() throws IOException { int level = obtainLockAndCheckState(); try { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index d4f0dfe3fa7b..8ebda0754be8 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -287,6 +287,7 @@ public void startCallableProcessing(Callable callable, Object... processingCo * via {@link #getConcurrentResultContext()} * @throws Exception if concurrent processing failed to start */ + @SuppressWarnings("NullAway") public void startCallableProcessing(final WebAsyncTask webAsyncTask, Object... processingContext) throws Exception { @@ -408,6 +409,7 @@ private void setConcurrentResultAndDispatch(@Nullable Object result) { * @see #getConcurrentResult() * @see #getConcurrentResultContext() */ + @SuppressWarnings("NullAway") public void startDeferredResultProcessing( final DeferredResult deferredResult, Object... processingContext) throws Exception { diff --git a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResourcePatternResolver.java b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResourcePatternResolver.java index 386f81fc3611..ae316792fdaa 100644 --- a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResourcePatternResolver.java +++ b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResourcePatternResolver.java @@ -104,6 +104,7 @@ protected Set doFindPathMatchingFileResources(Resource rootDirResource * @see ServletContextResource * @see jakarta.servlet.ServletContext#getResourcePaths */ + @SuppressWarnings("NullAway") protected void doRetrieveMatchingServletContextResources( ServletContext servletContext, String fullPattern, String dir, Set result) throws IOException { diff --git a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java index d66fe2f63eae..ba23f7859aed 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java +++ b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java @@ -171,6 +171,7 @@ public List getAllowedOrigins() { /** * Variant of {@link #setAllowedOrigins} for adding one origin at a time. */ + @SuppressWarnings("NullAway") public void addAllowedOrigin(@Nullable String origin) { if (origin == null) { return; @@ -244,6 +245,7 @@ public List getAllowedOriginPatterns() { * Variant of {@link #setAllowedOriginPatterns} for adding one origin at a time. * @since 5.3 */ + @SuppressWarnings("NullAway") public void addAllowedOriginPattern(@Nullable String originPattern) { if (originPattern == null) { return; diff --git a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java index 67202e75dfd3..b695c923f0ac 100644 --- a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java @@ -342,7 +342,7 @@ public String getShortLogMessage() { @Override public boolean equals(@Nullable Object other) { - return (this == other || (super.equals(other) && this.bean.equals(((HandlerMethod) other).bean))); + return (this == other || (super.equals(other) && other instanceof HandlerMethod otherMethod && this.bean.equals(otherMethod.bean))); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/multipart/support/AbstractMultipartHttpServletRequest.java b/spring-web/src/main/java/org/springframework/web/multipart/support/AbstractMultipartHttpServletRequest.java index a331034bf10f..9eacfdfa16ce 100644 --- a/spring-web/src/main/java/org/springframework/web/multipart/support/AbstractMultipartHttpServletRequest.java +++ b/spring-web/src/main/java/org/springframework/web/multipart/support/AbstractMultipartHttpServletRequest.java @@ -136,6 +136,7 @@ protected final void setMultipartFiles(MultiValueMap mult * lazily initializing it if necessary. * @see #initializeMultipart() */ + @SuppressWarnings("NullAway") protected MultiValueMap getMultipartFiles() { if (this.multipartFiles == null) { initializeMultipart(); diff --git a/spring-web/src/main/java/org/springframework/web/multipart/support/StandardMultipartHttpServletRequest.java b/spring-web/src/main/java/org/springframework/web/multipart/support/StandardMultipartHttpServletRequest.java index 4b7d21c6bfee..886fcbe15ef4 100644 --- a/spring-web/src/main/java/org/springframework/web/multipart/support/StandardMultipartHttpServletRequest.java +++ b/spring-web/src/main/java/org/springframework/web/multipart/support/StandardMultipartHttpServletRequest.java @@ -137,6 +137,7 @@ protected void initializeMultipart() { } @Override + @SuppressWarnings("NullAway") public Enumeration getParameterNames() { if (this.multipartParameterNames == null) { initializeMultipart(); @@ -157,6 +158,7 @@ public Enumeration getParameterNames() { } @Override + @SuppressWarnings("NullAway") public Map getParameterMap() { if (this.multipartParameterNames == null) { initializeMultipart(); diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index a645ea82c0e6..10659c9bd652 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -152,6 +152,7 @@ public void setCodecConfigurer(ServerCodecConfigurer codecConfigurer) { /** * Return the configured {@link ServerCodecConfigurer}. */ + @SuppressWarnings("NullAway") public ServerCodecConfigurer getCodecConfigurer() { if (this.codecConfigurer == null) { setCodecConfigurer(ServerCodecConfigurer.create()); diff --git a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java index ecc1557d6a82..98ecad3ae738 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java @@ -189,6 +189,7 @@ public InMemoryWebSession(Instant creationTime) { } @Override + @SuppressWarnings("NullAway") public String getId() { return this.id.get(); } @@ -224,6 +225,7 @@ public void start() { } @Override + @SuppressWarnings("NullAway") public boolean isStarted() { return this.state.get().equals(State.STARTED) || !getAttributes().isEmpty(); } @@ -252,6 +254,7 @@ public Mono invalidate() { } @Override + @SuppressWarnings("NullAway") public Mono save() { checkMaxSessionsLimit(); @@ -289,6 +292,7 @@ public boolean isExpired() { return isExpired(clock.instant()); } + @SuppressWarnings("NullAway") private boolean isExpired(Instant now) { if (this.state.get().equals(State.EXPIRED)) { return true; diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java index ca63327d242d..206be4e06be6 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java @@ -223,6 +223,7 @@ private static HttpMethod initHttpMethod(@Nullable HttpExchange typeAnnotation, } @Nullable + @SuppressWarnings("NullAway") private static String initUrl( @Nullable HttpExchange typeAnnotation, HttpExchange methodAnnotation, @Nullable StringValueResolver embeddedValueResolver) { diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java index 5a0e6f7b89ff..ec78080c7c30 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java @@ -251,7 +251,7 @@ public HttpServiceProxyFactory build() { this.exchangeAdapter, initArgumentResolvers(), this.embeddedValueResolver); } - @SuppressWarnings("DataFlowIssue") + @SuppressWarnings({"DataFlowIssue", "NullAway"}) private List initArgumentResolvers() { // Custom