RequestedContentTypeResolver does not ignore quality factor when filtering */* media types

[nbaars]

When you have two content type resolvers:

HeaderContentTypeResolver 
FixedContentTypeResolver 

And suppose the FixedContentTypeResolver returns application/json.

When you have the following controller definitions:

@RestController("/users")
public class MyController {

  @GetMapping()
  public Flux<String> f() { 
    ...
  }

  @GetMapping(produces = "text/csv")
  public Mono<ResponseEntity<String>> g() {
    ..
  }
}

So basically, we have two mappings one produces JSON and one a CSV file.

When you call this controller with:

curl -H 'Accept: */*' http://localhost:8080/users

It produces a JSON message. However, when you call this controller with:

curl -H 'Accept: */*;q=0.8' http://localhost:8080/users

All browsers do add a quality factor by default.
It defaults to whatever HttpMessageWriter can produce the type, in our case, a CSV mapper HttpMessageEncoder and it writes a CSV file with content type text/csv. One could argue the problem lies here, however looking at the code in RequestedContentTypeResolverBuilder:

public RequestedContentTypeResolver build() {
		List<RequestedContentTypeResolver> resolvers = (!this.candidates.isEmpty() ?
				this.candidates.stream().map(Supplier::get).collect(Collectors.toList()) :
				Collections.singletonList(new HeaderContentTypeResolver()));

		return exchange -> {
			for (RequestedContentTypeResolver resolver : resolvers) {
				List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
				if (mediaTypes.equals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST)) {
					continue;
				}
				return mediaTypes;
			}
			return RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST;
		};
	}

There is a special case for */* in which it tries the next content type resolver. But when it encounters */*;q=0.8 it does not work. The problem is that:

HeaderContentTypeResolver:

public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException {
		try {
			List<MediaType> mediaTypes = exchange.getRequest().getHeaders().getAccept();
			MediaType.sortBySpecificityAndQuality(mediaTypes);
			return (!CollectionUtils.isEmpty(mediaTypes) ? mediaTypes : MEDIA_TYPE_ALL_LIST);
		}
		catch (InvalidMediaTypeException ex) {
			String value = exchange.getRequest().getHeaders().getFirst("Accept");
			throw new NotAcceptableStatusException(
					"Could not parse 'Accept' header [" + value + "]: " + ex.getMessage());
		}
	}

does not apply the method MediaType#removeQualityValue, which is necessary to fire the continue block in the build method above.

The workaround for this issue is to add produces to the first controller method explicitly.

[bclozel]

Thanks for the analysis, but you're several steps ahead of us here. A sample application showing the behavior and a short note explaining what you would expect instead would be really helpful. Could you provide a minimal sample please?

[nbaars]

@bclozel simplified it to two test cases:

@Test
public void allMediaTypeWithTwoResolvers() {
	RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
	builder.resolver(new HeaderContentTypeResolver());
	builder.resolver(new FixedContentTypeResolver(MediaType.APPLICATION_JSON));
	RequestedContentTypeResolver resolver = builder.build();

	MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").accept(MediaType.ALL));
	List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
	assertThat(mediaTypes).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON));
}

@Test
public void allMediaTypeWithQualityFactorAndTwoResolvers() {
	RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
	builder.resolver(new HeaderContentTypeResolver());
	builder.resolver(new FixedContentTypeResolver(MediaType.APPLICATION_JSON));
	RequestedContentTypeResolver resolver = builder.build();

	MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").accept(MediaType.valueOf("*/*;q=0.8")));
	List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
	assertThat(mediaTypes).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON));
}

The last one fails it returns:

expected: [application/json]
 but was: [*/*;q=0.8]
org.opentest4j.AssertionFailedError: 
expected: [application/json]
 but was: [*/*;q=0.8]

The code in RequestedContentTypeResolver build() should use removeQualityValue from MediaType before applying the equals check.

[bclozel]

Thanks @nbaars for the report, this has been fixed in 6.0.x and 5.3.x and will be released tomorrow.