Simplify MediaTypeRequestMatcher construction

Fixes: gh-6612

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;
  * </pre>
  *
  * @author Rob Winch
+ * @author Dan Zheng
  * @since 3.2
  */
 
@@ -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
@@ -164,8 +183,7 @@ public final class MediaTypeRequestMatcher implements RequestMatcher {
 	 */
 	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;

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.<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 {
@@ -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();
+	}
 }