@RequestHeader negation expressions (e.g. !Accept=text/plain) are not applied [SPR-7346]

[spring-projects-issues]

Keith Donald opened SPR-7346 and commented

To reproduce:

• Checkout https://src.springsource.org/svn/spring-samples/mvc-showcase
• Deploy the application, view the root home page, and click "Mapping by not presence of header" link. You'll see "Mapped by regexp!", indicating the wrong @Controller method was matched. The method that should have been matched was MappingController.byHeaderNegation. The method looks like:

@RequestMapping(value="/mapping/header", method=RequestMethod.GET, headers="!Accept=text/plain")
public @ResponseBody String byHeaderNegation() {
	return "Mapped by path + method + header with negation!";
}

You can confirm using Firebug that the request information contains what should be required to match ("text/plain" is not an Accept value and the resource URL and HTTP method is correct).

---

Affects: 3.0.3

Attachments:

• ServletAnnotationMappingUtilsTest.java (8.18 kB)
• ServletAnnotationMappingUtilsTest.java (6.59 kB)
• SPR7346.patch (927 bytes)

Referenced from: commits f8425a5

4 votes, 2 watchers

[spring-projects-issues]

Arjen Poutsma commented

The negation operator only tests for header presence, not header value. I.e. !Accept=text/plain is interpreted as !Accept, and as such only applies to requests which don't have an Accept header. This is consistent with the way @RequestMapping.params works, and also documented, see http://static.springsource.org/spring/docs/3.0.x/javadoc-api/org/springframework/web/bind/annotation/RequestMapping.html#headers().

We could add the functionality that you request, but I would suggest using a different syntax (Accept!=text/plain) to make it easier to parse. I'm changing this issue from a bug to an improvement to do so.

[spring-projects-issues]

Arjen Poutsma commented

Implemented, using the != operator shown above ("content-type!=application/pdf"). Also implemented for RequestMapping.params().

[spring-projects-issues]

Keith Donald commented

The jsvadocs of request mapping indicate you should be able to test by value. Did you notice this?

[spring-projects-issues]

Arjen Poutsma commented

You are able to test by value, just not in combination with the ! operator:

Finally, "!My-Header" style expressions indicate that the specified header is not supposed to be present in the request.

Anyway, it's implemented now (using the != operator), so I wonder why we are even having this discussion.

[spring-projects-issues]

Christopher Taylor commented

negated header expressions are not evaluated correctly. I have these two controller methods:

@RequestMapping(method = RequestMethod.POST, headers="content-type!=application/xml")
public String create(...) {
...
}

@RequestMapping(method = RequestMethod.POST, headers="content-type=application/xml")
public ResponseEntity<String> create(...) {
...
}

If the content-type header on the request is set to application/xml, the first controller is invoked.

Using a debugger, I've traced this to faulty logic in ServletAnnotationMappingUtils.checkHeaders():
For the first method, the variable 'found' is true and the variable 'negated' is true for a request with the header "Content-Type: application/xml". However, the check

if(!found) {
    return negated;
}

causes the enclosing loop to continue, so that the method eventually ends up at the return true; on the last line, causing the wrong result to be returned.

ServletAnnotationMappingUtils.checkHeaders (correctly) also returns true for the second controller method. Control flow then returns to AnnotationMethodHandlerAdapter$ServletHandlerMethodResolver.resolveHnadlerMethod. RequestSpecificMappingInfoComparator and Collections.sort don't change the list of matches, so the first method is incorrectly returned as the controller method.

If I change my controller class and reverse the method definitions, the other create method is chosen. I suspect this bug can be reproduced by reversing the method definition order in ServletAnnotationControllerTests$NegatedContentTypeHeadersController, which should cause ServletAnnotationControllerTests.negatedContentTypeHeaders to fail.

[spring-projects-issues]

Eric Dalquist commented

Here is a unit test that verifies the logic problem in checkHeaders. The test also contains an updated version of checkHeaders that fixes the problem. I'll look at checking out the latest source as well and providing a true .patch file

[spring-projects-issues]

Eric Dalquist commented

Here is a patch to resolve the issue and an update to the test that also tests and fixes the negation operator for the media type headers

[spring-projects-issues]

Arjen Poutsma commented

Fixed as part of #12012