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

Fallback to defaultContentType if nothing more specific and producible has been specified in request [SPR-11114] #15740

Closed
spring-issuemaster opened this issue Nov 25, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Nov 25, 2013

Armel Bourgon opened SPR-11114 and commented

I am developing a web service that supports 2 types of response: json and jsonp. For convenience I want that it responds in json by default.

Here is how I configured my contentNegotiationManager:

<bean id="contentNegotiationManager" class="org.springframework.web.accept.ContentNegotiationManagerFactoryBean">
        <property name="favorPathExtension" value="false" />
        <property name="favorParameter" value="true" />
        <property name="mediaTypes">
            <value>
                json=application/json
                jsonp=application/x-javascript
            </value>
        </property>
        <property name="defaultContentType" value="application/json" />
</bean>

and my message converters:

<mvc:message-converters>
        <bean class="com.citygrid.sem.bid.api.jsonp.JsonpMessageConvertor">
            <property name="supportedMediaTypes">
                <list>
                    <value>application/x-javascript</value>
                </list>
            </property>
        </bean>
</mvc:message-converters>

What is interesting is what happen when no "format" parameter is defined in the request URL in addition to an Accept Header like this (generated by Firefox):

"text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"

In this case ContentNegotiationManager.resolveMediaTypes() returns:

[text/html, application/xhtml+xml, application/xml;q=0.9, */*;q=0.8]

Note that the specified default content type "application/json" is not in the list.
This is because ContentNegotiationManagerFactoryBean.afterPropertiesSet() puts the ContentNegotiationStrategy corresponding to the default content type at the end of the strategies list.
Then resolveMediaTypes() returns the media types returned by the first successful strategy (in this example HeaderContentNegotiationStrategy)

Because "application/json" is missing on the list returned by resolveMediaTypes() when AbstractMessageConverterMethodProcessor.writeWithMessageConverters() is called it tries to match the requested media types to the producible ones. In my example:

[application/x-javascript, application/json;charset=UTF-8, application/*+json;charset=UTF-8]

"*/*;q=0.8" and "application/x-javascript" are found compatible, as a consequence my JsonpMessageConvertor is called and the response is returned in jsonp despite the fact that I set defaultContentType to json.

I don't think it's a bug, defaultContentType is applied only if nothing but */* is defined in the request. Here there are specific content types (html and xml) in the request but none of them is supported, so the first declared producible type is returned.

It would be nice to have a mechanism to fallback to the defaultContentType if nothing more specific (and producible) has been set in the request.

For now I work around the issue by explicitly declaring the json converter before the jsonp one so that json is returned first in the list of producible types:

<mvc:message-converters>
        <bean class="org.springframework.http.converter.json.MappingJackson2HttpMessageConverter">
                <property name="supportedMediaTypes">
                    <list>
                        <value>application/json</value>
                    </list>
                </property>
        </bean>
        <bean class="com.citygrid.sem.bid.api.jsonp.JsonpMessageConvertor">
            <property name="supportedMediaTypes">
                <list>
                    <value>application/json</value>
                </list>
            </property>
        </bean>
</mvc:message-converters>

One solution I can think of is to modify ContentNegotiationManager.resolveMediaTypes() to returned all content types matched by all strategies (sorted with MediaType.sortBySpecificityAndQuality()) but I understand that it could slow down AbstractMessageConverterMethodProcessor.writeWithMessageConverters()... what do you guys think? I can put together a pull request with this solution if you want.


Issue Links:

  • #20208 Update Spring MVC to allow use of query param as replacement of path extension for content negotiation

Referenced from: commits 134ceac

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 9, 2014

Rossen Stoyanchev commented

I'm wondering why in the described scenario you don't turn off Accept header checking (i.e. ignoreAcceptHeader=true) and rely exclusively on a parameter or the default content type? You should then get the desired behavior, i.e. falling back on the default content type.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 9, 2014

Rossen Stoyanchev commented

What version of the framework are you using by the way?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 23, 2014

Armel Bourgon commented

Sorry long time without replying. I tested this with 3.2.4-RELEASE. But from what I see in github master branch nothing changed regarding this issue.

The idea was the following:

  1. People writing client should be able to choose between url parameter or header.
  2. It should be easy to test my webservice on a browser. So url parameter need to take precedence over header.
  3. A default response type (the recommended one for my WS) should be returned if requested ones are not supported.

Now that I thought more about the issue I see that the solution I proposed wouldn't work because of 2.
The problem with the current implementation is that when matching "requestedMediaTypes" and "producibleMediaTypes" */* should be treated as a special case.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 8, 2017

Rossen Stoyanchev commented

It's been a while since this was created but we are making some changes and reviewing the options in this area.

The purpose of ContentNegotiationStrategy was and still is to provide flexibility and plugability that should help with these types of scenarios. For example it would be trivial to create a custom strategy that delegates to HeaderContentNegotiationStrategy and FixedContentNegotiationStrategy and then combine into one list.

However currently ContentNegotiationConfigurer does not make it easy to configure your own list of strategies. Technically you could use ignoreAcceptHeader(true) together with defaultContentTypeStrategy(myCustomStrategy) but that's not ideal nor obvious. You could also use the advanced Spring MVC setup and override the ContentNegotiationManager bean method with then full control over the strategies to use but arguably this should be easy to do in ContentNegotiationConfigurer too.

So I'm scheduling for 5.0 RC2 to improve the configuration to make it easy to configure your own list of strategies ignoring the default setup.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 11, 2017

Rossen Stoyanchev commented

ContentNegotiationConfigurer and ContentNegotiationManagerFactoryBean now have methods that allow explicitly setting the strategies to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.