From 7139a877f41f864b3a318c0dadd3450fa67deb19 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 8 Feb 2022 13:22:44 +0100 Subject: [PATCH] Ensure toString() for synthesized annotations is source code compatible Since the introduction of synthesized annotation support in Spring Framework 4.2 (a.k.a., merged annotations), the toString() implementation attempted to align with the formatting used by the JDK itself. However, Class annotation attributes were formatted using Class#getName in Spring; whereas, the JDK used Class#toString up until JDK 9. In addition, JDK 9 introduced new formatting for toString() for annotations, apparently intended to align with the syntax used in the source code declaration of the annotation. However, JDK 9+ formats enum annotation attributes using Enum#toString instead of Enum#name, which can lead to issues if toString() is overridden in an enum. This commit updates the formatting used for synthesized annotations by ensuring that toString() generates a string that is compatible with the syntax of the originating source code, going beyond the changes made in JDK 9 by using Enum#name instead of Enum#toString. Closes gh-28015 --- ...izedMergedAnnotationInvocationHandler.java | 14 ++++-- .../annotation/MergedAnnotationsTests.java | 45 ++++++++++++++++--- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java index 0201e15e269..6838dc37797 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -195,18 +195,24 @@ final class SynthesizedMergedAnnotationInvocationHandler i } private String toString(Object value) { + if (value instanceof String) { + return '"' + value.toString() + '"'; + } + if (value instanceof Enum) { + return ((Enum) value).name(); + } if (value instanceof Class) { - return ((Class) value).getName(); + return ((Class) value).getName() + ".class"; } if (value.getClass().isArray()) { - StringBuilder builder = new StringBuilder("["); + StringBuilder builder = new StringBuilder("{"); for (int i = 0; i < Array.getLength(value); i++) { if (i > 0) { builder.append(", "); } builder.append(toString(Array.get(value, i))); } - builder.append(']'); + builder.append('}'); return builder.toString(); } return String.valueOf(value); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index 0c7708eee4b..0b92547863e 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -38,6 +38,7 @@ import java.util.stream.Stream; import javax.annotation.Resource; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.JRE; import org.springframework.core.Ordered; import org.springframework.core.annotation.MergedAnnotation.Adapt; @@ -1861,20 +1862,41 @@ class MergedAnnotationsTests { Method methodWithPath = WebController.class.getMethod("handleMappedWithPathAttribute"); RequestMapping webMappingWithAliases = methodWithPath.getAnnotation(RequestMapping.class); assertThat(webMappingWithAliases).isNotNull(); + Method methodWithPathAndValue = WebController.class.getMethod("handleMappedWithSamePathAndValueAttributes"); RequestMapping webMappingWithPathAndValue = methodWithPathAndValue.getAnnotation(RequestMapping.class); assertThat(methodWithPathAndValue).isNotNull(); + RequestMapping synthesizedWebMapping1 = MergedAnnotation.from(webMappingWithAliases).synthesize(); RequestMapping synthesizedWebMapping2 = MergedAnnotation.from(webMappingWithPathAndValue).synthesize(); + assertThat(webMappingWithAliases.toString()).isNotEqualTo(synthesizedWebMapping1.toString()); + + if (JRE.currentVersion().ordinal() > JRE.JAVA_8.ordinal()) { + // The unsynthesized annotation for handleMappedWithSamePathAndValueAttributes() + // should produce the same toString() results as synthesized annotations for + // handleMappedWithPathAttribute() on Java 9 or higher + assertToStringForWebMappingWithPathAndValue(webMappingWithPathAndValue); + } assertToStringForWebMappingWithPathAndValue(synthesizedWebMapping1); assertToStringForWebMappingWithPathAndValue(synthesizedWebMapping2); } private void assertToStringForWebMappingWithPathAndValue(RequestMapping webMapping) { - String prefix = "@" + RequestMapping.class.getName() + "("; - assertThat(webMapping.toString()).startsWith(prefix).contains("value=[/test]", - "path=[/test]", "name=bar", "method=", "[GET, POST]").endsWith(")"); + String string = webMapping.toString(); + + // Formatting common to Spring and JDK 9+ + assertThat(string) + .startsWith("@" + RequestMapping.class.getName() + "(") + .contains("value={\"/test\"}", "path={\"/test\"}", "name=\"bar\"", "clazz=java.lang.Object.class") + .endsWith(")"); + + if (webMapping instanceof SynthesizedAnnotation) { + assertThat(string).as("Spring uses Enum#name()").contains("method={GET, POST}"); + } + else { + assertThat(string).as("JDK uses Enum#toString()").contains("method={method: get, method: post}"); + } } @Test @@ -2941,7 +2963,17 @@ class MergedAnnotationsTests { } enum RequestMethod { - GET, POST + GET, + + POST; + + /** + * custom override to verify annotation toString() implementations. + */ + @Override + public String toString() { + return "method: " + name().toLowerCase(); + } } @Retention(RetentionPolicy.RUNTIME) @@ -2956,6 +2988,9 @@ class MergedAnnotationsTests { String[] path() default ""; RequestMethod[] method() default {}; + + // clazz is only used for testing annotation toString() implementations + Class clazz() default Object.class; } @Retention(RetentionPolicy.RUNTIME)