From ca353d678164a8a9750af68d1ec0753293414652 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 14 Jan 2022 14:32:12 -0700 Subject: [PATCH] Use noNullElements Collection#contains(null) does not work for all collection types Closes gh-10703 --- .../MediaTypeServerWebExchangeMatcher.java | 4 ++-- .../web/util/matcher/AndRequestMatcher.java | 4 ++-- .../web/util/matcher/OrRequestMatcher.java | 4 ++-- ...ediaTypeServerWebExchangeMatcherTests.java | 19 +++++++++++++++++- .../util/matcher/AndRequestMatcherTests.java | 19 +++++++++++++++++- .../util/matcher/OrRequestMatcherTests.java | 20 ++++++++++++++++++- 6 files changed, 61 insertions(+), 9 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcher.java b/web/src/main/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcher.java index 3ddc6b09df..654940be5d 100644 --- a/web/src/main/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcher.java +++ b/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 class MediaTypeServerWebExchangeMatcher implements ServerWebExchangeMatch */ public MediaTypeServerWebExchangeMatcher(Collection 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; } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java index 07682a183f..765c540493 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java +++ b/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 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; } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java index 92a2e2eb78..ae7dbaaaa5 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java +++ b/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 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; } diff --git a/web/src/test/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcherTests.java b/web/src/test/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcherTests.java index 3b651accce..f0bf13b0a1 100644 --- a/web/src/test/java/org/springframework/security/web/server/util/matcher/MediaTypeServerWebExchangeMatcherTests.java +++ b/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 class MediaTypeServerWebExchangeMatcherTests { assertThatIllegalArgumentException().isThrownBy(() -> new MediaTypeServerWebExchangeMatcher(types)); } + // gh-10703 + @Test + public void constructorListOfDoesNotThrowNullPointerException() { + List mediaTypes = new ArrayList(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 }; diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java index 08a2f851d1..c8c6e6239c 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java +++ b/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 class AndRequestMatcherTests { assertThatNullPointerException().isThrownBy(() -> new AndRequestMatcher((RequestMatcher[]) null)); } + // gh-10703 + @Test + public void constructorListOfDoesNotThrowNullPointer() { + List requestMatchers = new ArrayList( + 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)); diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java index c67b961de5..774bca7706 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java +++ b/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 class OrRequestMatcherTests { assertThatNullPointerException().isThrownBy(() -> new OrRequestMatcher((RequestMatcher[]) null)); } + // gh-10703 + @Test + public void constructorListOfDoesNotThrowNullPointer() { + // emulate List.of for pre-JDK 9 builds + List requestMatchers = new ArrayList( + 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));