AnnotationReadingVisitorUtils.getMergedAnnotationAttributes() mutates the map argument [SPR-11710]

[spring-projects-issues]

Gary Russell opened SPR-11710 and commented

Sam Brannen

List<AnnotationAttributes> attributesList = attributesMap.get(annotationType);
...
AnnotationAttributes results = attributesList.get(0);

...
    results.put(overridableAttributeName, value);

gets a hard reference to the map element and removes the value from that entry's list.

Consider:

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@MessagingGateway(defaultRequestChannel = "gatewayChannel", defaultReplyChannel = "gatewayChannel",
		defaultHeaders = @GatewayHeader(name = "foo", value = "FOO"))
public static @interface TestMessagingGateway {

	String defaultRequestChannel() default "";

}

and

MultiValueMap<String, Object> metaAttributesMap = importingClassMetadata.getAllAnnotationAttributes(MessagingGateway.class.getName());
Map<String, Object> annotationAttributes = importingClassMetadata.getAnnotationAttributes(MessagingGateway.class.getName());
metaAttributesMap = importingClassMetadata.getAllAnnotationAttributes(MessagingGateway.class.getName());

first call to getAllAnnotationAttributes():

{asyncExecutor=[], mapper=[], errorChannel=[], defaultRequestChannel=[gatewayChannel], ...

second call:

{name=[], asyncExecutor=[], mapper=[], errorChannel=[], defaultRequestChannel=[], ...

---

Affects: 4.0.3

Issue Links:

• Support meta-annotation attribute overrides in ASM-based annotation processing [SPR-11574] #16198 Support meta-annotation attribute overrides in ASM-based annotation processing
• AnnotationReadingVisitorUtils.getMergedAnnotationAttributes() overrides values it should not [SPR-11649] #16272 AnnotationReadingVisitorUtils.getMergedAnnotationAttributes() overrides values it should not

Referenced from: commits 82c32db, e1720d8

[spring-projects-issues]

Sam Brannen commented

Thanks for reporting this!

I'd actually already noticed it and fixed it in my solution for #16272 (see branch #16272 branch in my personal GitHub repository (direct link to commit)).

This is the corresponding change ("copy" being the key word):

// To start with, we populate the results with a copy of all attribute
// values from the target annotation.
AnnotationAttributes results = new AnnotationAttributes(attributesList.get(0));

This will go in once #16272 has been reviewed and merged into master, but we'll leave this issue open for the time being to ensure that it does not get overlooked.

Cheers,

Sam

[spring-projects-issues]

Sam Brannen commented

Fixed as described in the comments for GitHub commit e1720d8 (4.1) and GitHub commit 82c32db (4.0.4):

Don't mutate annotation metadata when merging attrs

Prior to this commit, invoking the getMergedAnnotationAttributes()
method in AnnotationReadingVisitorUtils resulted in mutation of the
internal state of the ASM-based annotation metadata supplied to the
method.

This commit fixes this issue by making a copy of the original
AnnotationAttributes for the target annotation before merging attribute
values from the meta-annotation hierarchy.

This commit also introduces a slight performance improvement by
avoiding duplicate processing of the attributes of the target
annotation.