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

negating specific media type in RequestMapping causes requests with no Accept header to fail [SPR-14299] #18871

Closed
spring-projects-issues opened this issue May 23, 2016 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Matt Fletcher opened SPR-14299 and commented

I am running on Spring/Spring MVC 3.2.11. I have a controller as follows:

@Controller
@RequestMapping("/cost")
public class CostGuideHomeController
{
   @RequestMapping(value="", method = RequestMethod.GET, produces = "!application/json")
   public String costGuideHome(final HttpServletRequest request, final ModelMap model)
   {
//...
   }
}

I have view resolvers as follows:

<bean class="org.springframework.web.servlet.view.ContentNegotiatingViewResolver">
      <property name="order" value="5"/>
      <property name="mediaTypes">
         <map>
            <entry key="json" value="application/json"/>
         </map>
      </property>
      <property name="defaultViews">
         <list>
            <bean class="org.springframework.web.servlet.view.json.MappingJacksonJsonView"/>
         </list>
      </property>
   </bean>

   <bean id="redirectViewResolver" class="com.**.spring.mvc.view.RedirectViewResolver">
      <description>
         Looks for view names with the 'redirect:' or 'permanent-redirect:'
         and performs either a 302 or 301 redirect, respectively.
      </description>
      <property name="order" value="10" />
	</bean>

   <bean id="tilesResolver" class="org.springframework.web.servlet.view.UrlBasedViewResolver">
      <description>Attempts to map a view name to a tiles definition.</description>
      <property name="order" value="15" />
      <property name="viewClass" value="org.springframework.web.servlet.view.tiles2.TilesView" />
	</bean>

   <bean id="viewResolver" class="org.springframework.web.servlet.view.InternalResourceViewResolver">
      <description>Look up view name in /WEB-INF/jsp directory</description>
      <property name="order" value="25" />
      <property name="viewClass" value="org.springframework.web.servlet.view.JstlView"/>
      <property name="prefix" value="/WEB-INF/jsp/"/>
      <property name="suffix" value=".jsp"/>
   </bean>

   <bean id="defaultViewResolver" class="org.springframework.web.servlet.view.InternalResourceViewResolver">
      <description>Default view resolver that attempts to look up the view name as a JSP.  This is really only needed
      because historically we didn't put jsps under /WEB-INF/jsp/*.  All spring controllers should be returning views
      that live in WEB-INF.  This view resolver handles all the old code that doesn't</description>
      <property name="order" value="30" />
      <property name="viewClass" value="org.springframework.web.servlet.view.JstlView"/>
   </bean>

I have been seeing requests to this controller with Accept=application/json. I want to disallow these. So I change the request mapping to the following:

@RequestMapping(value="", method = RequestMethod.GET, produces = "!application/json")

This does disallow requests with Accept=application/json, however it also appears to disallow requests which don't include a Accept header. Very unexpected behavior. Appears to be because the ProducesRequestCondition#getAcceptedMediaTypes returns a singleton list of MediaType.ALL. So it matches everything. Since the requestmapping has a negation, the subsequent check in ProducesRequestCondition#getMatchingCondition thinks that matching the request is not good... and I get back a 404 since no other handlers are configured for this endpoint.

I don't think we have a ContentNegotiationManager directly configured, and Spring appears to be using a default one that just looks at request headers.


Affects: 3.2.17

Referenced from: commits fc40643, 27215b5

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

We could consider a fix for 4.3. This late in the 3.2.x cycle (at 3.2.17 currently and planning to stop by the end of the year) I doubt we can justify back-porting such a change that has the potential for causing regressions.

Out of curiosity why do you want to prevent application/json exclusively? Normally these conditions are used to disambiguate between two or more controller methods. Is it specifically for this mapping only or in other places too? The answer can help determine a potential workaround for example.

@spring-projects-issues
Copy link
Collaborator Author

Matt Fletcher commented

Understood that this might not be worth fixing on 3.2.x. We're planning on upgrading soon anyhow... finally.

I want to prevent application/json exclusively because we have many endpoints where we don't want to directly expose the model data. So when we get random requests to these endpoints requesting json, we end up with 500 errors. Often due to serialization errors. So honestly, this is really only an issue for our initiative to reduce the number of 500 errors on our website. For endpoints where we are ok exposing the model data directly I am fixing serialization issues. But for endpoints where we don't want to expose it, I was hoping to use this functionality to tell the caller that their Accept header isn't supported by our endpoint.

Thanks,
Matt

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Have you considered catching HttpMessageNotWritableException and translating it to a status code of 406? That would allow you to handle it from a single place. You could also inject the HandlerMethod to see which controller method failed:

@ControllerAdvice
public class MyExceptionHandler {

    @ExceptionHandler
    public ResponseEntity<Void> handleException(HttpMediaTypeNotAcceptableException ex, HandlerMethod handlerMethod) {

        // ResponseEntity.status(HttpStatus.NOT_ACCEPTABLE).build();

    }

}

@spring-projects-issues
Copy link
Collaborator Author

Matt Fletcher commented

Thanks Rossen. I think that fits my immediate need perfectly.

If you still feel that this is a legitimate bug, please address in whatever version makes sense to you. If not, feel free to close it.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Yes we'll fix it for 4.3.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

This should be fixed for 4.3. I've also made a small optimization ensuring the requested content type is determined only once for any number of produces expressions.

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.3 GA milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants