From 6a93805bb4943ab095aeaa97d5cb985a4664ca91 Mon Sep 17 00:00:00 2001 From: jansupol Date: Thu, 15 Jun 2023 18:52:20 +0200 Subject: [PATCH 1/2] Parametrize ParamConverters to allow throwing IAE --- .../glassfish/jersey/CommonProperties.java | 18 ++- .../internal/inject/ParamConverters.java | 102 ++++++++++++----- docs/src/main/docbook/appendix-properties.xml | 79 +++++++------ docs/src/main/docbook/jersey.ent | 2 + .../common/internal/ParamConvertersTest.java | 105 ++++++++++++++++++ 5 files changed, 248 insertions(+), 58 deletions(-) create mode 100644 tests/e2e-core-common/src/test/java/org/glassfish/jersey/tests/e2e/common/internal/ParamConvertersTest.java diff --git a/core-common/src/main/java/org/glassfish/jersey/CommonProperties.java b/core-common/src/main/java/org/glassfish/jersey/CommonProperties.java index 762a08b050..bc76ba6f3a 100644 --- a/core-common/src/main/java/org/glassfish/jersey/CommonProperties.java +++ b/core-common/src/main/java/org/glassfish/jersey/CommonProperties.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2022 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2023 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -304,6 +304,22 @@ public final class CommonProperties { */ public static final String JSON_JACKSON_DISABLED_MODULES_SERVER = "jersey.config.server.json.jackson.disabled.modules"; + /** + *

+ * Force the {@link javax.ws.rs.ext.ParamConverter} to throw {@link IllegalArgumentException} as mandated in javadoc. + * Must be convertible to {@link Boolean} value. + *

+ *

+ * Internally the {@code Exception} is caught by Jersey and usually converted to {@code null}. + * Therefore, the default value is set to {@code false} to speed-up the conversion. + *

+ *

+ * The name of the configuration property is {@value}. + *

+ * @since 2.40 + */ + public static final String PARAM_CONVERTERS_THROW_IAE = "jersey.config.paramconverters.throw.iae"; + /** * Prevent instantiation. */ diff --git a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java index 699a2b784b..3d2aba7f0e 100644 --- a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java +++ b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java @@ -35,10 +35,12 @@ import javax.inject.Singleton; import javax.ws.rs.ProcessingException; import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Configuration; import javax.ws.rs.core.Context; import javax.ws.rs.ext.ParamConverter; import javax.ws.rs.ext.ParamConverterProvider; +import org.glassfish.jersey.CommonProperties; import org.glassfish.jersey.internal.LocalizationMessages; import org.glassfish.jersey.internal.util.ReflectionHelper; import org.glassfish.jersey.internal.util.collection.ClassTypePair; @@ -55,12 +57,32 @@ @Singleton public class ParamConverters { - private abstract static class AbstractStringReader implements ParamConverter { + private static class ParamConverterCompliance { + protected final boolean canReturnNull; + + private ParamConverterCompliance(boolean canReturnNull) { + this.canReturnNull = canReturnNull; + } + + protected T nullOrThrow() { + if (canReturnNull) { + return null; + } else { + throw new IllegalArgumentException(LocalizationMessages.METHOD_PARAMETER_CANNOT_BE_NULL("value")); + } + } + } + + private abstract static class AbstractStringReader extends ParamConverterCompliance implements ParamConverter { + + private AbstractStringReader(boolean canReturnNull) { + super(canReturnNull); + } @Override public T fromString(final String value) { if (value == null) { - return null; + return nullOrThrow(); } try { return _fromString(value); @@ -85,7 +107,7 @@ public T fromString(final String value) { @Override public String toString(final T value) throws IllegalArgumentException { if (value == null) { - return null; + return nullOrThrow(); } return value.toString(); } @@ -97,7 +119,11 @@ public String toString(final T value) throws IllegalArgumentException { * by invoking a single {@code String} parameter constructor on the target type. */ @Singleton - public static class StringConstructor implements ParamConverterProvider { + public static class StringConstructor extends ParamConverterCompliance implements ParamConverterProvider { + + private StringConstructor(boolean canReturnNull) { + super(canReturnNull); + } @Override public ParamConverter getConverter(final Class rawType, @@ -106,7 +132,7 @@ public ParamConverter getConverter(final Class rawType, final Constructor constructor = AccessController.doPrivileged(ReflectionHelper.getStringConstructorPA(rawType)); - return (constructor == null) ? null : new AbstractStringReader() { + return (constructor == null) ? null : new AbstractStringReader(canReturnNull) { @Override protected T _fromString(final String value) throws Exception { @@ -122,7 +148,11 @@ protected T _fromString(final String value) throws Exception { * by invoking a static {@code valueOf(String)} method on the target type. */ @Singleton - public static class TypeValueOf implements ParamConverterProvider { + public static class TypeValueOf extends ParamConverterCompliance implements ParamConverterProvider { + + private TypeValueOf(boolean canReturnNull) { + super(canReturnNull); + } @Override public ParamConverter getConverter(final Class rawType, @@ -131,7 +161,7 @@ public ParamConverter getConverter(final Class rawType, final Method valueOf = AccessController.doPrivileged(ReflectionHelper.getValueOfStringMethodPA(rawType)); - return (valueOf == null) ? null : new AbstractStringReader() { + return (valueOf == null) ? null : new AbstractStringReader(canReturnNull) { @Override public T _fromString(final String value) throws Exception { @@ -146,7 +176,11 @@ public T _fromString(final String value) throws Exception { * by invoking a static {@code fromString(String)} method on the target type. */ @Singleton - public static class TypeFromString implements ParamConverterProvider { + public static class TypeFromString extends ParamConverterCompliance implements ParamConverterProvider { + + private TypeFromString(boolean canReturnNull) { + super(canReturnNull); + } @Override public ParamConverter getConverter(final Class rawType, @@ -155,7 +189,7 @@ public ParamConverter getConverter(final Class rawType, final Method fromStringMethod = AccessController.doPrivileged(ReflectionHelper.getFromStringStringMethodPA(rawType)); - return (fromStringMethod == null) ? null : new AbstractStringReader() { + return (fromStringMethod == null) ? null : new AbstractStringReader(canReturnNull) { @Override public T _fromString(final String value) throws Exception { @@ -172,6 +206,10 @@ public T _fromString(final String value) throws Exception { @Singleton public static class TypeFromStringEnum extends TypeFromString { + private TypeFromStringEnum(boolean canReturnNull) { + super(canReturnNull); + } + @Override public ParamConverter getConverter(final Class rawType, final Type genericType, @@ -181,7 +219,11 @@ public ParamConverter getConverter(final Class rawType, } @Singleton - public static class CharacterProvider implements ParamConverterProvider { + public static class CharacterProvider extends ParamConverterCompliance implements ParamConverterProvider { + + private CharacterProvider(boolean canReturnNull) { + super(canReturnNull); + } @Override public ParamConverter getConverter(final Class rawType, @@ -192,7 +234,7 @@ public ParamConverter getConverter(final Class rawType, @Override public T fromString(String value) { if (value == null || value.isEmpty()) { - return null; + return CharacterProvider.this.nullOrThrow(); } if (value.length() == 1) { @@ -205,7 +247,7 @@ public T fromString(String value) { @Override public String toString(T value) { if (value == null) { - return null; + return CharacterProvider.this.nullOrThrow(); } return value.toString(); } @@ -222,7 +264,11 @@ public String toString(T value) { * {@link HttpDateFormat http date formatter} utility class. */ @Singleton - public static class DateProvider implements ParamConverterProvider { + public static class DateProvider extends ParamConverterCompliance implements ParamConverterProvider { + + private DateProvider(boolean canReturnNull) { + super(canReturnNull); + } @Override public ParamConverter getConverter(final Class rawType, @@ -233,7 +279,7 @@ public ParamConverter getConverter(final Class rawType, @Override public T fromString(final String value) { if (value == null) { - throw new IllegalArgumentException(LocalizationMessages.METHOD_PARAMETER_CANNOT_BE_NULL("value")); + return DateProvider.this.nullOrThrow(); } try { return rawType.cast(HttpDateFormat.readDate(value)); @@ -245,7 +291,7 @@ public T fromString(final String value) { @Override public String toString(final T value) throws IllegalArgumentException { if (value == null) { - return null; + return DateProvider.this.nullOrThrow(); } return value.toString(); } @@ -258,12 +304,13 @@ public String toString(final T value) throws IllegalArgumentException { * by invoking {@link ParamConverterProvider}. */ @Singleton - public static class OptionalCustomProvider implements ParamConverterProvider { + public static class OptionalCustomProvider extends ParamConverterCompliance implements ParamConverterProvider { // Delegates to this provider when the type of Optional is extracted. private final InjectionManager manager; - public OptionalCustomProvider(InjectionManager manager) { + public OptionalCustomProvider(InjectionManager manager, boolean canReturnNull) { + super(canReturnNull); this.manager = manager; } @@ -293,7 +340,7 @@ public T fromString(String value) { * In this case we don't send Optional.empty() because 'value' is not null. * But we return null because the provider didn't find how to parse it. */ - return null; + return nullOrThrow(); } } @@ -409,16 +456,21 @@ public static class AggregatedProvider implements ParamConverterProvider { */ @Inject public AggregatedProvider(@Context InjectionManager manager) { + Configuration configuration = manager.getInstance(Configuration.class); + boolean canThrowNull = !CommonProperties.getValue( + configuration.getProperties(), + CommonProperties.PARAM_CONVERTERS_THROW_IAE, + Boolean.FALSE); this.providers = new ParamConverterProvider[] { // ordering is important (e.g. Date provider must be executed before String Constructor // as Date has a deprecated String constructor - new DateProvider(), - new TypeFromStringEnum(), - new TypeValueOf(), - new CharacterProvider(), - new TypeFromString(), - new StringConstructor(), - new OptionalCustomProvider(manager), + new DateProvider(canThrowNull), + new TypeFromStringEnum(canThrowNull), + new TypeValueOf(canThrowNull), + new CharacterProvider(canThrowNull), + new TypeFromString(canThrowNull), + new StringConstructor(canThrowNull), + new OptionalCustomProvider(manager, canThrowNull), new OptionalProvider() }; } diff --git a/docs/src/main/docbook/appendix-properties.xml b/docs/src/main/docbook/appendix-properties.xml index 887b689630..8f619b4559 100644 --- a/docs/src/main/docbook/appendix-properties.xml +++ b/docs/src/main/docbook/appendix-properties.xml @@ -183,6 +183,21 @@ @since 2.36 + + &jersey.common.CommonProperties.PARAM_CONVERTERS_THROW_IAE;(Jersey 2.40 or later) + + + jersey.config.paramconverters.throw.iae + + + Force the &jaxrs.ext.ParamConverter; to throw IllegalArgumentException as mandated in javadoc. + Must be convertible to &lit.jdk6.Boolean; value. + + + Internally the Exception is caught by Jersey and usually converted to null + Therefore, the default value is set to &lit.false; to speed up the conversion. + + &jersey.logging.LoggingFeature.LOGGING_FEATURE_LOGGER_NAME; @@ -896,6 +911,38 @@ + + &jersey.client.ClientProperties.DIGESTAUTH_URI_CACHE_SIZELIMIT; + jersey.config.client.digestAuthUriCacheSizeLimit + + + The property defines a URI of a HTTP proxy the client connector should use. + + + + + &jersey.client.ClientProperties.EXPECT_100_CONTINUE; + jersey.config.client.request.expect.100.continue.processing + + + Allows for HTTP Expect:100-Continue being handled by the HttpUrlConnector (default Jersey + connector). Since 2.32 + + + + + &jersey.client.ClientProperties.EXPECT_100_CONTINUE_THRESHOLD_SIZE; + jersey.config.client.request.expect.100.continue.threshold.size + + + Property for threshold size for content length after which Expect:100-Continue header would be applied + before the main request. + Default threshold size (64kb) after which which Expect:100-Continue header would be applied before + the main request. + Since 2.32 + + + &jersey.client.ClientProperties.FEATURE_AUTO_DISCOVERY_DISABLE; jersey.config.client.disableAutoDiscovery @@ -1035,38 +1082,6 @@ - - &jersey.client.ClientProperties.DIGESTAUTH_URI_CACHE_SIZELIMIT; - jersey.config.client.digestAuthUriCacheSizeLimit - - - The property defines a URI of a HTTP proxy the client connector should use. - - - - - &jersey.client.ClientProperties.EXPECT_100_CONTINUE; - jersey.config.client.request.expect.100.continue.processing - - - Allows for HTTP Expect:100-Continue being handled by the HttpUrlConnector (default Jersey - connector). Since 2.32 - - - - - &jersey.client.ClientProperties.EXPECT_100_CONTINUE_THRESHOLD_SIZE; - jersey.config.client.request.expect.100.continue.threshold.size - - - Property for threshold size for content length after which Expect:100-Continue header would be applied - before the main request. - Default threshold size (64kb) after which which Expect:100-Continue header would be applied before - the main request. - Since 2.32 - - - &jersey.logging.LoggingFeature.LOGGING_FEATURE_LOGGER_NAME_CLIENT; diff --git a/docs/src/main/docbook/jersey.ent b/docs/src/main/docbook/jersey.ent index 733df58c84..990f12a01b 100644 --- a/docs/src/main/docbook/jersey.ent +++ b/docs/src/main/docbook/jersey.ent @@ -239,6 +239,7 @@ ExceptionMapper<E extends Throwable>"> MessageBodyReader<T>"> MessageBodyWriter<T>"> +ParamConverter<T>"> @Provider"> Providers"> RuntimeDelegate"> @@ -401,6 +402,7 @@ CommonProperties.JSON_JACKSON_DISABLED_MODULES" > CommonProperties.JSON_JACKSON_DISABLED_MODULES_CLIENT" > CommonProperties.JSON_JACKSON_DISABLED_MODULES_SERVER" > +CommonProperties.PARAM_CONVERTERS_THROW_IAE" > DisposableSupplier"> InjectionManager"> AbstractBinder"> diff --git a/tests/e2e-core-common/src/test/java/org/glassfish/jersey/tests/e2e/common/internal/ParamConvertersTest.java b/tests/e2e-core-common/src/test/java/org/glassfish/jersey/tests/e2e/common/internal/ParamConvertersTest.java new file mode 100644 index 0000000000..b415cf861a --- /dev/null +++ b/tests/e2e-core-common/src/test/java/org/glassfish/jersey/tests/e2e/common/internal/ParamConvertersTest.java @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.tests.e2e.common.internal; + +import org.glassfish.jersey.CommonProperties; +import org.glassfish.jersey.internal.inject.Bindings; +import org.glassfish.jersey.internal.inject.InjectionManager; +import org.glassfish.jersey.internal.inject.Injections; +import org.glassfish.jersey.internal.inject.ParamConverterConfigurator; +import org.glassfish.jersey.internal.inject.Providers; +import org.glassfish.jersey.model.internal.CommonConfig; +import org.glassfish.jersey.model.internal.ComponentBag; +import org.glassfish.jersey.model.internal.RankedComparator; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.ws.rs.core.Configuration; +import javax.ws.rs.ext.ParamConverter; +import javax.ws.rs.ext.ParamConverterProvider; +import java.util.Date; + +public class ParamConvertersTest { + private CommonConfig config; + + @BeforeEach + public void setUp() throws Exception { + config = new CommonConfig(null, ComponentBag.INCLUDE_ALL); + } + + @Test + public void paramConvertersReturnNull() { + ParamConverter converter = getParamConverter(String.class); + assertNull(String.class); + assertNull(Date.class); + assertNull(Character.class); + } + + @Test + public void paramConvertersThrow() { + config.property(CommonProperties.PARAM_CONVERTERS_THROW_IAE, true); + + assertThrow(String.class); + assertThrow(Date.class); + assertThrow(Character.class); + } + + private void assertNull(Class clazz) { + ParamConverter converter = getParamConverter(clazz); + Assertions.assertNotNull(converter); + Assertions.assertNull(converter.fromString(null)); + Assertions.assertNull(converter.toString(null)); + } + + private void assertThrow(Class clazz) { + ParamConverter converter = getParamConverter(clazz); + Assertions.assertNotNull(converter); + + try { + converter.fromString(null); + throw new RuntimeException("The IAE was not thrown"); + } catch (IllegalArgumentException illegalArgumentException) { + //expected + } + + try { + converter.toString(null); + throw new RuntimeException("The IAE was not thrown"); + } catch (IllegalArgumentException illegalArgumentException) { + //expected + } + } + + private ParamConverter getParamConverter(Class clazz) { + InjectionManager injectionManager = Injections.createInjectionManager(); + new ParamConverterConfigurator().init(injectionManager, null); + injectionManager.register(Bindings.service(config).to(Configuration.class)); + injectionManager.completeRegistration(); + + final Iterable allProviders = + Providers.getAllProviders(injectionManager, ParamConverterProvider.class, new RankedComparator<>()); + for (ParamConverterProvider provider : allProviders) { + ParamConverter converter = provider.getConverter(clazz, clazz, null); + if (converter != null) { + return converter; + } + } + return null; + } + +} From 6a902e370bc1861d89670038292b576608c9dd98 Mon Sep 17 00:00:00 2001 From: jansupol Date: Fri, 16 Jun 2023 15:33:38 +0200 Subject: [PATCH 2/2] Handle failing test Signed-off-by: jansupol --- .../glassfish/jersey/internal/inject/ParamConverters.java | 6 ++---- .../internal/inject/ParamConverterInternalTest.java | 8 ++++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java index 3d2aba7f0e..4a1ca86401 100644 --- a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java +++ b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java @@ -455,10 +455,8 @@ public static class AggregatedProvider implements ParamConverterProvider { * Create new aggregated {@link ParamConverterProvider param converter provider}. */ @Inject - public AggregatedProvider(@Context InjectionManager manager) { - Configuration configuration = manager.getInstance(Configuration.class); - boolean canThrowNull = !CommonProperties.getValue( - configuration.getProperties(), + public AggregatedProvider(@Context InjectionManager manager, @Context Configuration configuration) { + boolean canThrowNull = !CommonProperties.getValue(configuration.getProperties(), CommonProperties.PARAM_CONVERTERS_THROW_IAE, Boolean.FALSE); this.providers = new ParamConverterProvider[] { diff --git a/core-server/src/test/java/org/glassfish/jersey/server/internal/inject/ParamConverterInternalTest.java b/core-server/src/test/java/org/glassfish/jersey/server/internal/inject/ParamConverterInternalTest.java index 7ba24c81c7..6b54bd1d5c 100644 --- a/core-server/src/test/java/org/glassfish/jersey/server/internal/inject/ParamConverterInternalTest.java +++ b/core-server/src/test/java/org/glassfish/jersey/server/internal/inject/ParamConverterInternalTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2022 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2023 Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2018 Payara Foundation and/or its affiliates. * * This program and the accompanying materials are made available under the @@ -35,6 +35,7 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.QueryParam; +import javax.ws.rs.core.Configuration; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.ext.ParamConverter; import javax.ws.rs.ext.ParamConverterProvider; @@ -43,6 +44,8 @@ import org.glassfish.jersey.internal.inject.ParamConverters; import org.glassfish.jersey.internal.util.ReflectionHelper; import org.glassfish.jersey.internal.util.collection.ClassTypePair; +import org.glassfish.jersey.model.internal.CommonConfig; +import org.glassfish.jersey.model.internal.ComponentBag; import org.glassfish.jersey.server.ApplicationHandler; import org.glassfish.jersey.server.ContainerResponse; import org.glassfish.jersey.server.RequestContextBuilder; @@ -284,8 +287,9 @@ public void testLazyConverter() throws Exception { @Test public void testDateParamConverterIsChosenForDateString() { initiateWebApplication(); + final Configuration configuration = new CommonConfig(null, ComponentBag.EXCLUDE_EMPTY); final ParamConverter converter = - new ParamConverters.AggregatedProvider(null).getConverter(Date.class, Date.class, null); + new ParamConverters.AggregatedProvider(null, configuration).getConverter(Date.class, Date.class, null); assertEquals(ParamConverters.DateProvider.class, converter.getClass().getEnclosingClass(), "Unexpected date converter provider class");