Reject effectively private handler methods on CGLIB proxied controllers

spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

@@ -179,8 +179,6 @@ private StubArgumentResolver getStubResolver(int index) {
 		return (StubArgumentResolver) this.composite.getResolvers().get(index);
 	}
 
-
-
 	@SuppressWarnings("unused")
 	private static class Handler {
 
@@ -200,7 +198,6 @@ public String handleHandlerMethod(@Nullable HandlerMethod handlerMethod) {
 		}
 	}
 
-
 	private static class ExceptionRaisingArgumentResolver implements HandlerMethodArgumentResolver {
 
 		@Override

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java

@@ -19,10 +19,12 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 
@@ -71,6 +73,7 @@
  * @author Rossen Stoyanchev
  * @author Sam Brannen
  * @author Olga Maciaszek-Sharma
+ * @author Yongjun Hong
  * @since 3.1
  */
 @SuppressWarnings("removal")
@@ -183,13 +186,47 @@ protected boolean isHandler(Class<?> beanType) {
 	 * Uses type-level and method-level {@link RequestMapping @RequestMapping}
 	 * and {@link HttpExchange @HttpExchange} annotations to create the
 	 * {@link RequestMappingInfo}.
+	 * <p>For CGLIB proxy classes, additional validation is performed based on method visibility:
+	 * <ul>
+	 * <li>Private methods cannot be overridden and therefore cannot be used as handler methods.</li>
+	 * <li>Package-private methods from different packages are inaccessible and must be
+	 * changed to public or protected.</li>
+	 * </ul>
 	 * @return the created {@code RequestMappingInfo}, or {@code null} if the method
 	 * does not have a {@code @RequestMapping} or {@code @HttpExchange} annotation
 	 * @see #getCustomMethodCondition(Method)
 	 * @see #getCustomTypeCondition(Class)
 	 */
 	@Override
 	protected @Nullable RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
+		int modifiers = method.getModifiers();
+
+		if (isCglibProxy(handlerType)) {
+			if (Modifier.isPrivate(modifiers)) {
+				throw new IllegalStateException(
+						"Private method [" + method + "] on CGLIB proxy class [" + handlerType.getName() +
+								"] cannot be used as a request handler method because private methods cannot be overridden. " +
+								"Change the method to non-private visibility or use interface-based JDK proxying instead.");
+			}
+
+			if (!Modifier.isPublic(modifiers) && !Modifier.isProtected(modifiers)) {
+				Class<?> declaringClass = method.getDeclaringClass();
+				Package methodPackage = declaringClass.getPackage();
+				Package handlerPackage = handlerType.getPackage();
+
+				if (!Objects.equals(methodPackage, handlerPackage)) {
+					String methodPackageName = (methodPackage != null) ? methodPackage.getName() : "default package";
+					String handlerPackageName = (handlerPackage != null) ? handlerPackage.getName() : "default package";
+
+					throw new IllegalStateException(
+							"Package-private method [" + method + "] on CGLIB proxy class [" + declaringClass.getName() +
+									"] from package [" + methodPackageName + "] cannot be advised when used by handler class [" +
+									handlerType.getName() + "] from package [" + handlerPackageName + "] because it is effectively private. " +
+									"Either make the method public/protected or use interface-based JDK proxying instead.");
+				}
+			}
+		}
+
 		RequestMappingInfo info = createRequestMappingInfo(method);
 		if (info != null) {
 			RequestMappingInfo typeInfo = createRequestMappingInfo(handlerType);
@@ -207,6 +244,12 @@ protected boolean isHandler(Class<?> beanType) {
 		return info;
 	}
 
+
+	private boolean isCglibProxy(Class<?> beanType) {
+		return beanType.getName().contains("$$");
+	}
+
+
 	@Nullable String getPathPrefix(Class<?> handlerType) {
 		for (Map.Entry<String, Predicate<Class<?>>> entry : this.pathPrefixes.entrySet()) {
 			if (entry.getValue().test(handlerType)) {

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java

@@ -29,6 +29,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.provider.Arguments;
 
+import org.springframework.cglib.proxy.Enhancer;
+import org.springframework.cglib.proxy.NoOp;
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.http.MediaType;
 import org.springframework.stereotype.Controller;
@@ -69,6 +71,7 @@
  * @author Rossen Stoyanchev
  * @author Sam Brannen
  * @author Olga Maciaszek-Sharma
+ * @author Yongjun Hong
  */
 class RequestMappingHandlerMappingTests {
 
@@ -461,6 +464,59 @@ void requestBodyAnnotationFromImplementationOverridesInterface() throws Exceptio
 		assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build());
 	}
 
+	@Test
+	void privateMethodOnCglibProxyShouldThrowException() {
+		RequestMappingHandlerMapping mapping = createMapping();
+
+		Class<?> handlerType = PrivateMethodController.class;
+		Method method;
+		try {
+			method = handlerType.getDeclaredMethod("privateMethod");
+		}
+		catch (NoSuchMethodException ex) {
+			throw new IllegalStateException(ex);
+		}
+
+		final Class<?> proxyClass = createProxyClass(handlerType);
+
+		assertThatIllegalStateException()
+				.isThrownBy(() -> mapping.getMappingForMethod(method, proxyClass))
+				.withMessageContaining("Private method")
+				.withMessageContaining("cannot be used as a request handler method");
+	}
+
+	@Test
+	void protectedMethodShouldNotThrowException() throws Exception {
+		RequestMappingHandlerMapping mapping = createMapping();
+
+		Class<?> handlerType = ProtectedMethodController.class;
+		Method method = handlerType.getDeclaredMethod("protectedMethod");
+		final Class<?> proxyClass = createProxyClass(handlerType);
+
+		RequestMappingInfo info = mapping.getMappingForMethod(method, proxyClass);
+
+		assertThat(info.getPatternValues()).containsOnly("/protected");
+	}
+
+	private Class<?> createProxyClass(Class<?> targetClass) {
+		Enhancer enhancer = new Enhancer();
+		enhancer.setSuperclass(targetClass);
+		enhancer.setCallbackTypes(new Class[]{NoOp.class});
+
+		return enhancer.createClass();
+	}
+
+	@Controller
+	static class PrivateMethodController {
+		@RequestMapping("/private")
+		private void privateMethod() {}
+	}
+
+	@Controller
+	static class ProtectedMethodController {
+		@RequestMapping("/protected")
+		protected void protectedMethod() {}
+	}
 
 	private static RequestMappingHandlerMapping createMapping() {
 		RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();