Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrize ParamConverters to allow throwing IAE #5349

Merged
merged 2 commits into from
Jun 20, 2023
Merged

Parametrize ParamConverters to allow throwing IAE #5349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a93805
Parametrize ParamConverters to allow throwing IAE
jansupol Jun 15, 2023
6a902e3
Handle failing test
jansupol Jun 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion core-common/src/main/java/org/glassfish/jersey/CommonProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -304,6 +304,22 @@ public final class CommonProperties {
*/
public static final String JSON_JACKSON_DISABLED_MODULES_SERVER = "jersey.config.server.json.jackson.disabled.modules";

/**
* <p>
* Force the {@link javax.ws.rs.ext.ParamConverter} to throw {@link IllegalArgumentException} as mandated in javadoc.
* Must be convertible to {@link Boolean} value.
* </p>
* <p>
* 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.
* </p>
* <p>
* The name of the configuration property is <tt>{@value}</tt>.
* </p>
* @since 2.40
*/
public static final String PARAM_CONVERTERS_THROW_IAE = "jersey.config.paramconverters.throw.iae";

/**
* Prevent instantiation.
*/
Expand Down
102 changes: 76 additions & 26 deletions core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -55,12 +57,32 @@
@Singleton
public class ParamConverters {

private abstract static class AbstractStringReader<T> implements ParamConverter<T> {
private static class ParamConverterCompliance {
protected final boolean canReturnNull;

private ParamConverterCompliance(boolean canReturnNull) {
this.canReturnNull = canReturnNull;
}

protected <T> T nullOrThrow() {
if (canReturnNull) {
return null;
} else {
throw new IllegalArgumentException(LocalizationMessages.METHOD_PARAMETER_CANNOT_BE_NULL("value"));
}
}
}

private abstract static class AbstractStringReader<T> extends ParamConverterCompliance implements ParamConverter<T> {

private AbstractStringReader(boolean canReturnNull) {
super(canReturnNull);
}

@Override
public T fromString(final String value) {
if (value == null) {
return null;
return nullOrThrow();
}
try {
return _fromString(value);
Expand All @@ -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();
}
Expand All @@ -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 <T> ParamConverter<T> getConverter(final Class<T> rawType,
Expand All @@ -106,7 +132,7 @@ public <T> ParamConverter<T> getConverter(final Class<T> rawType,

final Constructor constructor = AccessController.doPrivileged(ReflectionHelper.getStringConstructorPA(rawType));

return (constructor == null) ? null : new AbstractStringReader<T>() {
return (constructor == null) ? null : new AbstractStringReader<T>(canReturnNull) {

@Override
protected T _fromString(final String value) throws Exception {
Expand All @@ -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 <T> ParamConverter<T> getConverter(final Class<T> rawType,
Expand All @@ -131,7 +161,7 @@ public <T> ParamConverter<T> getConverter(final Class<T> rawType,

final Method valueOf = AccessController.doPrivileged(ReflectionHelper.getValueOfStringMethodPA(rawType));

return (valueOf == null) ? null : new AbstractStringReader<T>() {
return (valueOf == null) ? null : new AbstractStringReader<T>(canReturnNull) {

@Override
public T _fromString(final String value) throws Exception {
Expand All @@ -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 <T> ParamConverter<T> getConverter(final Class<T> rawType,
Expand All @@ -155,7 +189,7 @@ public <T> ParamConverter<T> getConverter(final Class<T> rawType,

final Method fromStringMethod = AccessController.doPrivileged(ReflectionHelper.getFromStringStringMethodPA(rawType));

return (fromStringMethod == null) ? null : new AbstractStringReader<T>() {
return (fromStringMethod == null) ? null : new AbstractStringReader<T>(canReturnNull) {

@Override
public T _fromString(final String value) throws Exception {
Expand All @@ -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 <T> ParamConverter<T> getConverter(final Class<T> rawType,
final Type genericType,
Expand All @@ -181,7 +219,11 @@ public <T> ParamConverter<T> getConverter(final Class<T> rawType,
}

@Singleton
public static class CharacterProvider implements ParamConverterProvider {
public static class CharacterProvider extends ParamConverterCompliance implements ParamConverterProvider {

private CharacterProvider(boolean canReturnNull) {
super(canReturnNull);
}

@Override
public <T> ParamConverter<T> getConverter(final Class<T> rawType,
Expand All @@ -192,7 +234,7 @@ public <T> ParamConverter<T> getConverter(final Class<T> rawType,
@Override
public T fromString(String value) {
if (value == null || value.isEmpty()) {
return null;
return CharacterProvider.this.nullOrThrow();
}

if (value.length() == 1) {
Expand All @@ -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();
}
Expand All @@ -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 <T> ParamConverter<T> getConverter(final Class<T> rawType,
Expand All @@ -233,7 +279,7 @@ public <T> ParamConverter<T> getConverter(final Class<T> 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));
Expand All @@ -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();
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -408,17 +455,20 @@ public static class AggregatedProvider implements ParamConverterProvider {
* Create new aggregated {@link ParamConverterProvider param converter provider}.
*/
@Inject
public AggregatedProvider(@Context InjectionManager manager) {
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[] {
// 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()
};
}
Expand Down
8 changes: 6 additions & 2 deletions ...src/test/java/org/glassfish/jersey/server/internal/inject/ParamConverterInternalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Date> 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");
Expand Down
Loading