From 56c4d2d4ca371a9a09c654f7eb6ddbfbdcdf0488 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 7 Feb 2025 13:01:04 +0000 Subject: [PATCH 1/4] Improve HandlerMethod#resolvedFromHandlerMethod initialization Ensure the original instance is always the one returned no matter how many times the HandlerMethod is re-created. Make the constructor protected to allow subclasses to re-create the HandlerMethod as the concrete subclass. See gh-34375 --- .../web/method/HandlerMethod.java | 11 ++++++++--- .../web/method/HandlerMethodTests.java | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) 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 0ef3a1bc001..44c7cd2066e 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 @@ -181,9 +181,13 @@ public class HandlerMethod extends AnnotatedMethod { } /** - * Re-create HandlerMethod with additional input. + * Re-create new HandlerMethod instance that copies the given HandlerMethod + * but replaces the handler, and optionally checks for the presence of + * validation annotations. + *

Subclasses can override this to ensure that a HandlerMethod is of the + * same type if re-created. */ - private HandlerMethod(HandlerMethod handlerMethod, @Nullable Object handler, boolean initValidateFlags) { + protected HandlerMethod(HandlerMethod handlerMethod, @Nullable Object handler, boolean initValidateFlags) { super(handlerMethod); this.bean = (handler != null ? handler : handlerMethod.bean); this.beanFactory = handlerMethod.beanFactory; @@ -197,7 +201,8 @@ public class HandlerMethod extends AnnotatedMethod { handlerMethod.validateReturnValue); this.responseStatus = handlerMethod.responseStatus; this.responseStatusReason = handlerMethod.responseStatusReason; - this.resolvedFromHandlerMethod = handlerMethod; + this.resolvedFromHandlerMethod = (handlerMethod.resolvedFromHandlerMethod != null ? + handlerMethod.resolvedFromHandlerMethod : handlerMethod); this.description = handlerMethod.toString(); } diff --git a/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java b/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java index 660dfa06d6e..8d1f825028c 100644 --- a/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java @@ -25,6 +25,7 @@ import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.Size; import org.junit.jupiter.api.Test; +import org.springframework.context.support.StaticApplicationContext; import org.springframework.util.ClassUtils; import org.springframework.validation.annotation.Validated; @@ -79,6 +80,23 @@ class HandlerMethodTests { assertThat(handlerMethod.createWithResolvedBean()).isSameAs(handlerMethod); } + @Test + void resolvedFromHandlerMethod() { + StaticApplicationContext context = new StaticApplicationContext(); + context.registerSingleton("myClass", MyClass.class); + + MyClass target = new MyClass(); + Method method = ClassUtils.getMethod(target.getClass(), "addPerson", (Class[]) null); + + HandlerMethod hm1 = new HandlerMethod("myClass", context.getBeanFactory(), method); + HandlerMethod hm2 = hm1.createWithValidateFlags(); + HandlerMethod hm3 = hm2.createWithResolvedBean(); + + assertThat(hm1.getResolvedFromHandlerMethod()).isNull(); + assertThat(hm2.getResolvedFromHandlerMethod()).isSameAs(hm1); + assertThat(hm3.getResolvedFromHandlerMethod()).isSameAs(hm1); + } + private static void testValidateArgs(Object target, List methodNames, boolean expected) { for (String methodName : methodNames) { assertThat(getHandlerMethod(target, methodName).shouldValidateArguments()).isEqualTo(expected); From ff49b0b683d94a163916aaccf09a3a217eda76aa Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 7 Feb 2025 13:06:18 +0000 Subject: [PATCH 2/4] Align AnnotatedMethod#equals and #hashcode See gh-34375 --- .../springframework/core/annotation/AnnotatedMethod.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java index e50e79bf900..fd8fb107997 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -214,8 +214,8 @@ public class AnnotatedMethod { @Override public boolean equals(@Nullable Object other) { - return (this == other || (other != null && getClass() == other.getClass() && - this.method.equals(((AnnotatedMethod) other).method))); + return (this == other || (other instanceof AnnotatedMethod otherHandlerMethod && + this.method.equals(otherHandlerMethod.method))); } @Override From 84992536c5dcc007d2bda4f345b04f56a845a1b9 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 7 Feb 2025 13:10:33 +0000 Subject: [PATCH 3/4] Defer initialization of HandlerMethod validation flags Re-create the HandlerMethod only after the original is used as a key in the CORS lookup map. Closes gh-34375 --- .../handler/AbstractHandlerMethodMapping.java | 9 ++-- .../handler/HandlerMethodMappingTests.java | 42 ++++++++++++++++++- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java index 62caefd2f1b..3ec91acdebf 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -636,9 +636,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap HandlerMethod handlerMethod = createHandlerMethod(handler, method); validateMethodMapping(handlerMethod, mapping); - // Enable method validation, if applicable - handlerMethod = handlerMethod.createWithValidateFlags(); - Set directPaths = AbstractHandlerMethodMapping.this.getDirectPaths(mapping); for (String path : directPaths) { this.pathLookup.add(path, mapping); @@ -657,6 +654,10 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap this.corsLookup.put(handlerMethod, corsConfig); } + // Init validation flags + // We do this strictly after using the original instance in the CORS lookups + handlerMethod = handlerMethod.createWithValidateFlags(); + this.registry.put(mapping, new MappingRegistration<>(mapping, handlerMethod, directPaths, name, corsConfig != null)); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java index 75d7eca575f..9e3b3cf4151 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -292,6 +292,23 @@ public class HandlerMethodMappingTests { HandlerMethod handlerMethod = this.mapping.getHandlerInternal(new MockHttpServletRequest("GET", key)); } + @Test + void registerCustomHandlerMethod() throws Exception { + this.mapping.setCustomerHandlerMethod(true); + this.mapping.registerMapping("/foo", this.handler, this.handler.getClass().getMethod("corsHandlerMethod")); + + MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", "/foo"); + request.addParameter("abort", "true"); + request.addHeader(HttpHeaders.ORIGIN, "https://domain.com"); + request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET"); + + MockHttpServletResponse response = new MockHttpServletResponse(); + + HandlerExecutionChain chain = this.mapping.getHandler(request); + + assertThat(chain).isNotNull(); + assertThat(response.getStatus()).isEqualTo(200); + } private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping { @@ -302,6 +319,8 @@ public class HandlerMethodMappingTests { private final List matches = new ArrayList<>(); + private boolean customerHandlerMethod; + public MyHandlerMethodMapping() { setHandlerMethodMappingNamingStrategy(new SimpleMappingNamingStrategy()); } @@ -326,6 +345,16 @@ public class HandlerMethodMappingTests { return methodName.startsWith("handler") ? methodName : null; } + public void setCustomerHandlerMethod(boolean customerHandlerMethod) { + this.customerHandlerMethod = customerHandlerMethod; + } + + @Override + protected HandlerMethod createHandlerMethod(Object handler, Method method) { + return (this.customerHandlerMethod ? + new CustomHandlerMethod(handler, method) : super.createHandlerMethod(handler, method)); + } + @Override protected CorsConfiguration initCorsConfiguration(Object handler, Method method, String mapping) { CrossOrigin crossOrigin = AnnotatedElementUtils.findMergedAnnotation(method, CrossOrigin.class); @@ -355,6 +384,7 @@ public class HandlerMethodMappingTests { } + private static class SimpleMappingNamingStrategy implements HandlerMethodMappingNamingStrategy { @Override @@ -363,6 +393,16 @@ public class HandlerMethodMappingTests { } } + + private static class CustomHandlerMethod extends HandlerMethod { + + public CustomHandlerMethod(Object bean, Method method) { + super(bean, method); + } + + } + + @Controller static class MyHandler { From 1d7cb4fc134f91e97c23529ae18bd22761daf77c Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 7 Feb 2025 13:20:16 +0000 Subject: [PATCH 4/4] Defer initialization of HandlerMethod validation flags Re-create the HandlerMethod only after the original is used as a key in the CORS lookup map. Closes gh-34379 --- .../web/util/ServletRequestPathUtils.java | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java b/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java index 4c627896589..279ceaae749 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java +++ b/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -179,6 +179,27 @@ public abstract class ServletRequestPathUtils { request.getAttribute(UrlPathHelper.PATH_ATTRIBUTE) != null); } + /** + * Check if the Servlet is mapped by a path prefix, and if so return that + * path prefix. + * @param request the current request + * @return the prefix, or {@code null} if the Servlet is not mapped by prefix + * @since 6.2.3 + */ + @Nullable + public static String getServletPathPrefix(HttpServletRequest request) { + HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); + mapping = (mapping != null ? mapping : request.getHttpServletMapping()); + if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { + String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); + servletPath = (servletPath != null ? servletPath : request.getServletPath()); + servletPath = (servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) : servletPath); + return servletPath; + } + return null; + } + + /** * Simple wrapper around the default {@link RequestPath} implementation that @@ -251,22 +272,11 @@ public abstract class ServletRequestPathUtils { String requestUri = (String) request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE); requestUri = (requestUri != null ? requestUri : request.getRequestURI()); String servletPathPrefix = getServletPathPrefix(request); - return (StringUtils.hasText(servletPathPrefix) ? - new ServletRequestPath(new PathElements(requestUri, request.getContextPath(), servletPathPrefix)) : - RequestPath.parse(requestUri, request.getContextPath())); - } - - @Nullable - private static String getServletPathPrefix(HttpServletRequest request) { - HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); - mapping = (mapping != null ? mapping : request.getHttpServletMapping()); - if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { - String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); - servletPath = (servletPath != null ? servletPath : request.getServletPath()); - servletPath = (servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) : servletPath); - return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8); + if (!StringUtils.hasLength(servletPathPrefix)) { + return RequestPath.parse(requestUri, request.getContextPath()); } - return null; + servletPathPrefix = UriUtils.encodePath(servletPathPrefix, StandardCharsets.UTF_8); + return new ServletRequestPath(new PathElements(requestUri, request.getContextPath(), servletPathPrefix)); } record PathElements(String rawPath, @Nullable String contextPath, String servletPathPrefix) {