From a9a86cd826a61bff3633457f7c4d04673722b140 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Tue, 26 Mar 2019 10:41:20 +0800 Subject: [PATCH] Simplify MediaTypeRequestMatcher construction Fixes: gh-6612 --- .../util/matcher/MediaTypeRequestMatcher.java | 24 ++- .../matcher/MediaTypeRequestMatcherTests.java | 173 +++++++++++++++++- 2 files changed, 189 insertions(+), 8 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcher.java index 3f03582a75..22565ab8f1 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcher.java @@ -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. @@ -30,6 +30,7 @@ import org.springframework.http.MediaType; 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; /** @@ -135,6 +136,7 @@ import org.springframework.web.context.request.ServletWebRequest; * * * @author Rob Winch + * @author Dan Zheng * @since 3.2 */ @@ -145,6 +147,23 @@ public final class MediaTypeRequestMatcher implements RequestMatcher { private boolean useEquals; private Set 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 matchingMediaTypes) { + this(new HeaderContentNegotiationStrategy(), matchingMediaTypes); + } /** * Creates an instance * @param contentNegotiationStrategy the {@link ContentNegotiationStrategy} to use @@ -164,8 +183,7 @@ public final class MediaTypeRequestMatcher implements RequestMatcher { */ public MediaTypeRequestMatcher(ContentNegotiationStrategy contentNegotiationStrategy, Collection 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; diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcherTests.java index 8bcf8dc6d2..19d320ea9c 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/MediaTypeRequestMatcherTests.java @@ -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. @@ -28,16 +28,16 @@ import org.junit.Test; 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 { @@ -53,8 +53,9 @@ public class MediaTypeRequestMatcherTests { } @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) @@ -79,6 +80,16 @@ public class MediaTypeRequestMatcherTests { Collections. emptyList()); } + @Test(expected = IllegalArgumentException.class) + public void constructorWhenEmptyMediaTypeThenIAE() { + new MediaTypeRequestMatcher(); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenEmptyMediaTypeCollectionThenIAE() { + new MediaTypeRequestMatcher(Collections. emptyList()); + } + @Test public void negotiationStrategyThrowsHMTNAE() throws HttpMediaTypeNotAcceptableException { @@ -91,6 +102,7 @@ public class MediaTypeRequestMatcherTests { @Test public void mediaAllMatches() throws Exception { + when(negotiationStrategy.resolveMediaTypes(any(NativeWebRequest.class))) .thenReturn(Arrays.asList(MediaType.ALL)); @@ -102,6 +114,77 @@ public class MediaTypeRequestMatcherTests { 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))) @@ -133,6 +216,14 @@ public class MediaTypeRequestMatcherTests { 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 { @@ -143,6 +234,15 @@ public class MediaTypeRequestMatcherTests { 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 @@ -156,6 +256,15 @@ public class MediaTypeRequestMatcherTests { 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 { @@ -168,6 +277,15 @@ public class MediaTypeRequestMatcherTests { 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))) @@ -178,6 +296,15 @@ public class MediaTypeRequestMatcherTests { 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))) @@ -189,6 +316,15 @@ public class MediaTypeRequestMatcherTests { 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 @@ -201,6 +337,15 @@ public class MediaTypeRequestMatcherTests { 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 { @@ -212,6 +357,15 @@ public class MediaTypeRequestMatcherTests { 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 { @@ -224,4 +378,13 @@ public class MediaTypeRequestMatcherTests { 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(); + } }