Use noNullElements

Collection#contains(null) does not work for all collection types

Closes gh-10703

Author: jzheaux

web/src/main/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -66,7 +66,7 @@ public MediaTypeServerWebExchangeMatcher(MediaType... matchingMediaTypes) {
 	 */
 	public MediaTypeServerWebExchangeMatcher(Collection<MediaType> matchingMediaTypes) {
 		Assert.notEmpty(matchingMediaTypes, "matchingMediaTypes cannot be null");
-		Assert.isTrue(!matchingMediaTypes.contains(null),
+		Assert.noNullElements(matchingMediaTypes,
 				() -> "matchingMediaTypes cannot contain null. Got " + matchingMediaTypes);
 		this.matchingMediaTypes = matchingMediaTypes;
 	}

web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -45,7 +45,7 @@ public final class AndRequestMatcher implements RequestMatcher {
 	 */
 	public AndRequestMatcher(List<RequestMatcher> requestMatchers) {
 		Assert.notEmpty(requestMatchers, "requestMatchers must contain a value");
-		Assert.isTrue(!requestMatchers.contains(null), "requestMatchers cannot contain null values");
+		Assert.noNullElements(requestMatchers, "requestMatchers cannot contain null values");
 		this.requestMatchers = requestMatchers;
 	}
 

web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -40,7 +40,7 @@ public final class OrRequestMatcher implements RequestMatcher {
 	 */
 	public OrRequestMatcher(List<RequestMatcher> requestMatchers) {
 		Assert.notEmpty(requestMatchers, "requestMatchers must contain a value");
-		Assert.isTrue(!requestMatchers.contains(null), "requestMatchers cannot contain null values");
+		Assert.noNullElements(requestMatchers, "requestMatchers cannot contain null values");
 		this.requestMatchers = requestMatchers;
 	}
 

web/src/test/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcherTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -16,6 +16,8 @@
 
 package org.springframework.security.web.server.util.matcher;
 
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -43,6 +45,21 @@ public void constructorMediaTypeArrayWhenNullThenThrowsIllegalArgumentException(
 		assertThatIllegalArgumentException().isThrownBy(() -> new MediaTypeServerWebExchangeMatcher(types));
 	}
 
+	// gh-10703
+	@Test
+	public void constructorListOfDoesNotThrowNullPointerException() {
+		List<MediaType> mediaTypes = new ArrayList<MediaType>(Arrays.asList(MediaType.ALL)) {
+			@Override
+			public boolean contains(Object o) {
+				if (o == null) {
+					throw new NullPointerException();
+				}
+				return super.contains(o);
+			}
+		};
+		new MediaTypeServerWebExchangeMatcher(mediaTypes);
+	}
+
 	@Test
 	public void constructorMediaTypeArrayWhenContainsNullThenThrowsIllegalArgumentException() {
 		MediaType[] types = { null };

web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.security.web.util.matcher;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -55,6 +56,22 @@ public void constructorNullArray() {
 		assertThatNullPointerException().isThrownBy(() -> new AndRequestMatcher((RequestMatcher[]) null));
 	}
 
+	// gh-10703
+	@Test
+	public void constructorListOfDoesNotThrowNullPointer() {
+		List<RequestMatcher> requestMatchers = new ArrayList<RequestMatcher>(
+				Arrays.asList(AnyRequestMatcher.INSTANCE)) {
+			@Override
+			public boolean contains(Object o) {
+				if (o == null) {
+					throw new NullPointerException();
+				}
+				return super.contains(o);
+			}
+		};
+		new AndRequestMatcher(requestMatchers);
+	}
+
 	@Test
 	public void constructorArrayContainsNull() {
 		assertThatIllegalArgumentException().isThrownBy(() -> new AndRequestMatcher((RequestMatcher) null));

web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.security.web.util.matcher;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -55,6 +56,23 @@ public void constructorNullArray() {
 		assertThatNullPointerException().isThrownBy(() -> new OrRequestMatcher((RequestMatcher[]) null));
 	}
 
+	// gh-10703
+	@Test
+	public void constructorListOfDoesNotThrowNullPointer() {
+		// emulate List.of for pre-JDK 9 builds
+		List<RequestMatcher> requestMatchers = new ArrayList<RequestMatcher>(
+				Arrays.asList(AnyRequestMatcher.INSTANCE)) {
+			@Override
+			public boolean contains(Object o) {
+				if (o == null) {
+					throw new NullPointerException();
+				}
+				return super.contains(o);
+			}
+		};
+		new OrRequestMatcher(requestMatchers);
+	}
+
 	@Test
 	public void constructorArrayContainsNull() {
 		assertThatIllegalArgumentException().isThrownBy(() -> new OrRequestMatcher((RequestMatcher) null));