Skip to content

Commit

Permalink
Ignore quality factor when filtering out "*/*"
Browse files Browse the repository at this point in the history
Prior to this commit, the `RequestedContentTypeResolverBuilder` would
create a `RequestedContentTypeResolver` that internally delegates to a
list of resolvers. Each resolver would either return the list of
requested media types, or a singleton list with the "*/*" media type; in
this case this signals that the resolver cannot find a specific media
type requested and that we should continue with the next resolver in the
list.

Media Types returned by resolvers can contain parameters, such as the
quality factor. If the HTTP client requests "*/*;q=0.8", the
`HeaderContentTypeResolver` will return this as a singleton list. While
this has been resolved from the request, such a media type should not be
selected over other media types that could be returned by other
resolvers.

This commit changes the `RequestedContentTypeResolverBuilder` so that it
does not select "*/*;q=0.8" as the requested media type, but instead
continues delegating to other resolvers in the list. This means we need
to remove the quality factor before comparing it to the "*/*" for
equality check.

Fixes gh-29915
  • Loading branch information
bclozel committed Mar 15, 2023
1 parent 1de36ab commit 2a5eab4
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public RequestedContentTypeResolver build() {
return exchange -> {
for (RequestedContentTypeResolver resolver : resolvers) {
List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
if (mediaTypes.equals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST)) {
if (isMediaTypeAll(mediaTypes)) {
continue;
}
return mediaTypes;
Expand All @@ -102,6 +102,11 @@ public RequestedContentTypeResolver build() {
};
}

private boolean isMediaTypeAll(List<MediaType> mediaTypes) {
return mediaTypes.size() == 1
&& mediaTypes.get(0).removeQualityValue().equals(MediaType.ALL);
}


/**
* Helper to create and configure {@link ParameterContentTypeResolver}.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -34,8 +34,7 @@
public class RequestedContentTypeResolverBuilderTests {

@Test
public void defaultSettings() throws Exception {

void defaultSettings() {
RequestedContentTypeResolver resolver = new RequestedContentTypeResolverBuilder().build();
MockServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("/flower").accept(MediaType.IMAGE_GIF));
Expand All @@ -45,8 +44,7 @@ public void defaultSettings() throws Exception {
}

@Test
public void parameterResolver() throws Exception {

void parameterResolver() {
RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
builder.parameterResolver().mediaType("json", MediaType.APPLICATION_JSON);
RequestedContentTypeResolver resolver = builder.build();
Expand All @@ -58,8 +56,7 @@ public void parameterResolver() throws Exception {
}

@Test
public void parameterResolverWithCustomParamName() throws Exception {

void parameterResolverWithCustomParamName() {
RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
builder.parameterResolver().mediaType("json", MediaType.APPLICATION_JSON).parameterName("s");
RequestedContentTypeResolver resolver = builder.build();
Expand All @@ -71,8 +68,7 @@ public void parameterResolverWithCustomParamName() throws Exception {
}

@Test // SPR-10513
public void fixedResolver() throws Exception {

void fixedResolver() {
RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
builder.fixedResolver(MediaType.APPLICATION_JSON);
RequestedContentTypeResolver resolver = builder.build();
Expand All @@ -84,8 +80,7 @@ public void fixedResolver() throws Exception {
}

@Test // SPR-12286
public void resolver() throws Exception {

void resolver() {
RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder();
builder.resolver(new FixedContentTypeResolver(MediaType.APPLICATION_JSON));
RequestedContentTypeResolver resolver = builder.build();
Expand All @@ -99,4 +94,17 @@ public void resolver() throws Exception {
assertThat(mediaTypes).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON));
}

@Test
void removeQualityFactorForMediaTypeAllChecks() {
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));
}

}

0 comments on commit 2a5eab4

Please sign in to comment.