Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring MVC @RequestHeader value is ignored [SPR-17617] #22149

Closed
spring-issuemaster opened this issue Dec 21, 2018 · 12 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Dec 21, 2018

zsenyeg opened SPR-17617 and commented

RequestHeaderMethodArgumentResolver creates RequestHeaderNamedValueInfo information from annotation using only name value of RequestHeader annotation.

In generated codes for example swagger codegen 3, only the value attribute is filled, and because of that in the rest interface the name of the method argument will be used, that is might be different than the name (or value) of the RequestHeader annotation.

For example:

@RequestHeader(value = "Consent-ID",required = false) String consentId

 

Name attribute of annotation is not filled, because of that method argument name consentId will be used, and won't found among the http request headers.

 


Affects: 5.1.2

Attachments:

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 21, 2018

Juergen Hoeller commented

The name attribute is retrieved from a synthesized annotation there, taking into an account that value is an alias for it. This works fine in our unit tests, as far as I can see.

Could you narrow the cause a bit further, possibly providing a test case for it?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 2, 2019

zsenyeg commented

Hey,

 

I've made a small junit test about what i mean. Can you check the attached file pls?

java version: 1.8

spring boot version: 2.1.0.RELEASE

mockito-core version: 2.23.0

 

Cheers,

zsenyeg

[^RequestHeaderMethodArgumentResolverTest.java]

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Jan 18, 2019

The example doesn't help. The original description says the value attribute is filled out, the test uses a mock which does not do that. Please, provide an example with an actual annotated controller, or a snippet that shows what that declaration looks like.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 25, 2019

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@zsenyeg

This comment has been minimized.

Copy link

@zsenyeg zsenyeg commented Jan 29, 2019

Hey,

I just made a little runnable example, that fail as the previous unit test. The attached zip file contains a maven idea project, you can start it with the usual mvn spring-boot:run command.

The sample rest service provides two get endpoints:
http://localhost:8080/test/good
http://localhost:8080/test/bad

Test both endpoints with the following http request header: Consent-ID with any value.

cheers,
zsenyeg
springrest.zip

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Mar 11, 2019

Thanks for the sample.

It looks like when @RequestHeader(value="...") is declared on the class method, both the value and the name attributes are filled out. When the same is declared on an interface, only the value attribute is filled out. @sbrannen any idea why?

Note that support for Spring MVC method parameter annotations on an interface was only added recently in 5.1 with #15682.

@rstoyanchev rstoyanchev changed the title MVC @RequestHeader value is ignored [SPR-17617] Spring MVC `@RequestHeader` value is ignored [SPR-17617] Mar 11, 2019
@rstoyanchev rstoyanchev changed the title Spring MVC `@RequestHeader` value is ignored [SPR-17617] Spring MVC @RequestHeader value is ignored [SPR-17617] Mar 11, 2019
@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Mar 11, 2019

It looks like when @RequestHeader(value="...") is declared on the class method, both the value and the name attributes are filled out. When the same is declared on an interface, only the value attribute is filled out. @sbrannen any idea why?

I'll have to look into it before I can comment on that.

@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Mar 11, 2019

At a glance, it appears that the infrastructure there is using MethodParameter instead of SynthesizingMethodParameter, and the lookup is via RequestHeader.name() in RequestHeaderMethodArgumentResolver.RequestHeaderNamedValueInfo.

@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Mar 11, 2019

I don't yet have a test in place, but I assume it's due to the fact that org.springframework.web.method.HandlerMethod.getInterfaceParameterAnnotations() does not synthesize annotations retrieved from methods in interfaces.

I'll add a test and fix it. 😉

@sbrannen sbrannen self-assigned this Mar 11, 2019
@sbrannen sbrannen added the type: bug label Mar 11, 2019
@sbrannen sbrannen added this to the 5.1.6 milestone Mar 11, 2019
@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Mar 11, 2019

@sbrannen it's using the MethodParameter that's passed in which was initialized as a SynthesizingMethodParameter in HandlerMethod. I don't think that's the issue.

Switching to 5.1.5 I can no longer reproduce the issue, most likely thanks to recent changes by @jhoeller in HandlerMethod.

@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Mar 11, 2019

OK. I see that adaptAnnotation() eventually gets called on annotations merged from the results of getInterfaceParameterAnnotations() (thereby synthesizing them).

So perhaps @jhoeller has indeed already fixed this.

@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Mar 11, 2019

Yep, this was the fix: c58da71#diff-4e35372004bab14ceec2182f61179bafL497

Closing this issue as as duplicate of #21992.

@sbrannen sbrannen closed this Mar 11, 2019
@sbrannen sbrannen removed this from the 5.1.6 milestone Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.