Skip to content

Simplify MediaTypeRequestMatcher construction #6648

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Simplify MediaTypeRequestMatcher construction #6648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions web/src/main/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 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 @@ -30,6 +30,7 @@
import org.springframework.util.Assert;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.accept.ContentNegotiationStrategy;
import org.springframework.web.accept.HeaderContentNegotiationStrategy;
import org.springframework.web.context.request.ServletWebRequest;

/**
Expand Down Expand Up @@ -135,6 +136,7 @@
* </pre>
*
* @author Rob Winch
* @author Dan Zheng
* @since 3.2
*/

Expand All @@ -145,6 +147,23 @@ public final class MediaTypeRequestMatcher implements RequestMatcher {
private boolean useEquals;
private Set<MediaType> ignoredMediaTypes = Collections.emptySet();

/**
* Creates an instance
* @param matchingMediaTypes the {@link MediaType} that will make the http request.
* @since 5.2
*/
public MediaTypeRequestMatcher(MediaType... matchingMediaTypes) {
this(new HeaderContentNegotiationStrategy(), Arrays.asList(matchingMediaTypes));
}

/**
* Creates an instance
* @param matchingMediaTypes the {@link MediaType} that will make the http request.
* @since 5.2
*/
public MediaTypeRequestMatcher(Collection<MediaType> matchingMediaTypes) {
this(new HeaderContentNegotiationStrategy(), matchingMediaTypes);
}
/**
* Creates an instance
* @param contentNegotiationStrategy the {@link ContentNegotiationStrategy} to use
Expand All @@ -164,8 +183,7 @@ public MediaTypeRequestMatcher(ContentNegotiationStrategy contentNegotiationStra
*/
public MediaTypeRequestMatcher(ContentNegotiationStrategy contentNegotiationStrategy,
Collection<MediaType> matchingMediaTypes) {
Assert.notNull(contentNegotiationStrategy,
"ContentNegotiationStrategy cannot be null");
Assert.notNull(contentNegotiationStrategy, "ContentNegotiationStrategy cannot be null");
Assert.notEmpty(matchingMediaTypes, "matchingMediaTypes cannot be null or empty");
this.contentNegotiationStrategy = contentNegotiationStrategy;
this.matchingMediaTypes = matchingMediaTypes;
Expand Down
173 changes: 168 additions & 5 deletions ...test/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcherTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 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 @@ -28,16 +28,16 @@
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

import org.springframework.http.MediaType;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.accept.ContentNegotiationStrategy;
import org.springframework.web.context.request.NativeWebRequest;

/**
* @author Rob Winch
*
* @author Dan Zheng
*/
@RunWith(MockitoJUnitRunner.class)
public class MediaTypeRequestMatcherTests {
Expand All @@ -53,8 +53,9 @@ public void setup() {
}

@Test(expected = IllegalArgumentException.class)
public void constructorNullCNSVarargs() {
new MediaTypeRequestMatcher(null, MediaType.ALL);
public void constructorWhenNullCNSThenIAE() {
ContentNegotiationStrategy c = null;
new MediaTypeRequestMatcher(c, MediaType.ALL);
}

@Test(expected = IllegalArgumentException.class)
Expand All @@ -79,6 +80,16 @@ public void constructorEmtpyMediaTypes() {
Collections.<MediaType> emptyList());
}

@Test(expected = IllegalArgumentException.class)
public void constructorWhenEmptyMediaTypeThenIAE() {
new MediaTypeRequestMatcher();
}

@Test(expected = IllegalArgumentException.class)
public void constructorWhenEmptyMediaTypeCollectionThenIAE() {
new MediaTypeRequestMatcher(Collections.<MediaType> emptyList());
}

@Test
public void negotiationStrategyThrowsHMTNAE()
throws HttpMediaTypeNotAcceptableException {
Expand All @@ -91,6 +102,7 @@ public void negotiationStrategyThrowsHMTNAE()

@Test
public void mediaAllMatches() throws Exception {

when(negotiationStrategy.resolveMediaTypes(any(NativeWebRequest.class)))
.thenReturn(Arrays.asList(MediaType.ALL));

Expand All @@ -102,6 +114,77 @@ public void mediaAllMatches() throws Exception {
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderAsteriskThenAll() throws Exception {
request.addHeader("Accept", "*/*");
matcher = new MediaTypeRequestMatcher(MediaType.ALL);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderAsteriskThenAnyone() throws Exception {
request.addHeader("Accept", "*/*");
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderAsteriskThenAllInCollection() throws Exception {
request.addHeader("Accept", "*/*");
matcher = new MediaTypeRequestMatcher(Collections.singleton(MediaType.ALL));
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderAsteriskThenAnyoneInCollection() throws Exception {
request.addHeader("Accept", "*/*");
matcher = new MediaTypeRequestMatcher(Collections.singleton(MediaType.TEXT_HTML));
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenNoAcceptHeaderThenAll() throws Exception {
request.removeHeader("Accept");
// if not set Accept, it is match all
matcher = new MediaTypeRequestMatcher(MediaType.ALL);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenNoAcceptHeaderThenAnyone() throws Exception {
request.removeHeader("Accept");
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenSingleAcceptHeaderThenOne() throws Exception {
request.addHeader("Accept", "text/html");
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenSingleAcceptHeaderThenOneWithCollection() throws Exception {
request.addHeader("Accept", "text/html");
matcher = new MediaTypeRequestMatcher(Collections.singleton(MediaType.TEXT_HTML));
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenMultipleAcceptHeaderThenMatchMultiple() throws Exception {
request.addHeader("Accept", "text/html, application/xhtml+xml, application/xml;q=0.9");
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML, MediaType.APPLICATION_XHTML_XML, MediaType.APPLICATION_XML);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenMultipleAcceptHeaderThenAnyoneInCollection() throws Exception {
request.addHeader("Accept", "text/html, application/xhtml+xml, application/xml;q=0.9");
matcher = new MediaTypeRequestMatcher(Arrays.asList(MediaType.APPLICATION_XHTML_XML));
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void multipleMediaType() throws HttpMediaTypeNotAcceptableException {
when(negotiationStrategy.resolveMediaTypes(any(NativeWebRequest.class)))
Expand Down Expand Up @@ -133,6 +216,14 @@ public void resolveTextPlainMatchesTextAll()
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderIsTextThenMediaTypeAllIsMatched() {
request.addHeader("Accept", MediaType.TEXT_PLAIN_VALUE);

matcher = new MediaTypeRequestMatcher(new MediaType("text", "*"));
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void resolveTextAllMatchesTextPlain()
throws HttpMediaTypeNotAcceptableException {
Expand All @@ -143,6 +234,15 @@ public void resolveTextAllMatchesTextPlain()
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void matchWhenAcceptHeaderIsTextWildcardThenMediaTypeTextIsMatched() {
request.addHeader("Accept", "text/*");

matcher = new MediaTypeRequestMatcher(MediaType.TEXT_PLAIN);
assertThat(matcher.matches(request)).isTrue();
}


// useEquals

@Test
Expand All @@ -156,6 +256,15 @@ public void useEqualsResolveTextAllMatchesTextPlain()
assertThat(matcher.matches(request)).isFalse();
}

@Test
public void useEqualsWhenTrueThenMediaTypeTextIsNotMatched() {
request.addHeader("Accept", "text/*");

matcher = new MediaTypeRequestMatcher(MediaType.TEXT_PLAIN);
matcher.setUseEquals(true);
assertThat(matcher.matches(request)).isFalse();
}

@Test
public void useEqualsResolveTextPlainMatchesTextAll()
throws HttpMediaTypeNotAcceptableException {
Expand All @@ -168,6 +277,15 @@ public void useEqualsResolveTextPlainMatchesTextAll()
assertThat(matcher.matches(request)).isFalse();
}

@Test
public void useEqualsWhenTrueThenMediaTypeTextAllIsNotMatched() {
request.addHeader("Accept", MediaType.TEXT_PLAIN_VALUE);

matcher = new MediaTypeRequestMatcher(new MediaType("text", "*"));
matcher.setUseEquals(true);
assertThat(matcher.matches(request)).isFalse();
}

@Test
public void useEqualsSame() throws HttpMediaTypeNotAcceptableException {
when(negotiationStrategy.resolveMediaTypes(any(NativeWebRequest.class)))
Expand All @@ -178,6 +296,15 @@ public void useEqualsSame() throws HttpMediaTypeNotAcceptableException {
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void useEqualsWhenTrueThenMediaTypeIsMatchedWithEqualString() {
request.addHeader("Accept", MediaType.TEXT_PLAIN_VALUE);

matcher = new MediaTypeRequestMatcher(MediaType.TEXT_PLAIN);
matcher.setUseEquals(true);
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void useEqualsWithCustomMediaType() throws HttpMediaTypeNotAcceptableException {
when(negotiationStrategy.resolveMediaTypes(any(NativeWebRequest.class)))
Expand All @@ -189,6 +316,15 @@ public void useEqualsWithCustomMediaType() throws HttpMediaTypeNotAcceptableExce
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void useEqualsWhenTrueThenCustomMediaTypeIsMatched() {
request.addHeader("Accept", "text/unique");

matcher = new MediaTypeRequestMatcher(new MediaType("text", "unique"));
matcher.setUseEquals(true);
assertThat(matcher.matches(request)).isTrue();
}

// ignoreMediaTypeAll

@Test
Expand All @@ -201,6 +337,15 @@ public void mediaAllIgnoreMediaTypeAll() throws HttpMediaTypeNotAcceptableExcept
assertThat(matcher.matches(request)).isFalse();
}

@Test
public void ignoredMediaTypesWhenAllThenAnyoneIsNotMatched() {
request.addHeader("Accept", MediaType.ALL_VALUE);
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
matcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL));

assertThat(matcher.matches(request)).isFalse();
}

@Test
public void mediaAllAndTextHtmlIgnoreMediaTypeAll()
throws HttpMediaTypeNotAcceptableException {
Expand All @@ -212,6 +357,15 @@ public void mediaAllAndTextHtmlIgnoreMediaTypeAll()
assertThat(matcher.matches(request)).isTrue();
}

@Test
public void ignoredMediaTypesWhenAllAndTextThenTextCanBeMatched() {
request.addHeader("Accept", MediaType.ALL_VALUE + ", " + MediaType.TEXT_HTML_VALUE);
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
matcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL));

assertThat(matcher.matches(request)).isTrue();
}

@Test
public void mediaAllQ08AndTextPlainIgnoreMediaTypeAll()
throws HttpMediaTypeNotAcceptableException {
Expand All @@ -224,4 +378,13 @@ public void mediaAllQ08AndTextPlainIgnoreMediaTypeAll()

assertThat(matcher.matches(request)).isFalse();
}

@Test
public void ignoredMediaTypesWhenAllThenQ08WithTextIsNotMatched() {
request.addHeader("Accept", MediaType.TEXT_PLAIN + ", */*;q=0.8");
matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
matcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL));

assertThat(matcher.matches(request)).isFalse();
}
}