This is an automated email from the ASF dual-hosted git repository.
benw pushed a commit to branch TAP5-2763
in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit c55d039af792c7b2052473d3219f3acd71888c1a
Author: Ben Weidig <b...@netzgut.net>
AuthorDate: Sun Sep 24 11:36:39 2023 +0200
TAP5-2763: improve @RequestParameter blank handling
@RequestParamter now also has "boolean treatBlankAsNull() default false" to
better handl blank values.
A value might be blank, even if it's supposed to be a null coming from the
client, as any parameter in a request set to null will end up as blank on the
server side.
This might lead to coercion problems if no sensible coercer from blank to
the target type exists.
To keep the handling as close to the original, an additional annotation
parameter made more sense than changing the overall handling.
---
.../tapestry5/annotations/RequestParameter.java | 22 ++++++++---
.../internal/transform/OnEventWorker.java | 44 +++++++++++++++-------
.../src/test/app1/RequestParameterDemo.tml | 6 +++
.../integration/app1/RequestParameterTests.java | 19 ++++++++++
.../app1/pages/RequestParameterDemo.java | 34 +++++++++++++++--
5 files changed, 103 insertions(+), 22 deletions(-)
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/annotations/RequestParameter.java
b/tapestry-core/src/main/java/org/apache/tapestry5/annotations/RequestParameter.java
index be48db3fc..4dd571dc7 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/annotations/RequestParameter.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/annotations/RequestParameter.java
@@ -12,17 +12,19 @@
package org.apache.tapestry5.annotations;
-import org.apache.tapestry5.http.services.Request;
-import org.apache.tapestry5.internal.transform.OnEventWorker;
-import org.apache.tapestry5.ioc.annotations.UseWith;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static
org.apache.tapestry5.ioc.annotations.AnnotationUseContext.COMPONENT;
+import static org.apache.tapestry5.ioc.annotations.AnnotationUseContext.MIXIN;
+import static org.apache.tapestry5.ioc.annotations.AnnotationUseContext.PAGE;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
-import static java.lang.annotation.ElementType.PARAMETER;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import static org.apache.tapestry5.ioc.annotations.AnnotationUseContext.*;
+import org.apache.tapestry5.http.services.Request;
+import org.apache.tapestry5.internal.transform.OnEventWorker;
+import org.apache.tapestry5.ioc.annotations.UseWith;
/**
* Annotation that may be placed on parameters of event handler methods.
@@ -54,4 +56,12 @@ public @interface RequestParameter
* implementation.
*/
boolean allowBlank() default false;
+
+ /**
+ * Treats a blank value as null before passing it along to the appropriate
{@link org.apache.tapestry5.ValueEncoder}
+ * implementation. This only affects request parameters with {@link
#allowBlank()} set to true.
+ *
+ * @since 5.8.4
+ */
+ boolean treatBlankAsNull() default false;
}
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/OnEventWorker.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/OnEventWorker.java
index 5468bc487..f35018a62 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/OnEventWorker.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/OnEventWorker.java
@@ -329,7 +329,7 @@ public class OnEventWorker implements
ComponentClassTransformWorker2
String parameterName = parameterAnnotation.value();
providers.add(createQueryParameterProvider(method, i,
parameterName, type,
- parameterAnnotation.allowBlank()));
+ parameterAnnotation.allowBlank(),
parameterAnnotation.treatBlankAsNull()));
continue;
}
@@ -615,7 +615,7 @@ public class OnEventWorker implements
ComponentClassTransformWorker2
}
private EventHandlerMethodParameterProvider
createQueryParameterProvider(PlasticMethod method, final int parameterIndex,
final String parameterName,
-
final String parameterTypeName, final boolean allowBlank)
+
final String parameterTypeName, final boolean allowBlank, final boolean
treatBlankAsNull)
{
final String methodIdentifier = method.getMethodIdentifier();
@@ -639,23 +639,33 @@ public class OnEventWorker implements
ComponentClassTransformWorker2
String parameterValue =
request.getParameter(parameterName);
- if (!allowBlank && InternalUtils.isBlank(parameterValue))
- throw new RuntimeException(String.format(
- "The value for query parameter '%s' was blank,
but a non-blank value is needed.",
- parameterName));
+ if (InternalUtils.isBlank(parameterValue))
+ {
+ if (!allowBlank)
+ {
+ throw new RuntimeException(String.format(
+ "The value for query parameter '%s' was
blank, but a non-blank value is needed.",
+ parameterName));
+ }
+
+ if (treatBlankAsNull)
+ {
+ parameterValue = null;
+ }
+ }
Object value;
if (!isArray)
{
- value = coerce(parameterName, parameterType,
parameterValue, valueEncoder, allowBlank);
+ value = coerce(parameterName, parameterType,
parameterValue, valueEncoder, allowBlank, treatBlankAsNull);
} else
{
String[] parameterValues =
request.getParameters(parameterName);
Object[] array = (Object[])
Array.newInstance(parameterType, parameterValues.length);
for (int i = 0; i < parameterValues.length; i++)
{
- array[i] = coerce(parameterName, parameterType,
parameterValues[i], valueEncoder, allowBlank);
+ array[i] = coerce(parameterName, parameterType,
parameterValues[i], valueEncoder, allowBlank, treatBlankAsNull);
}
value = array;
}
@@ -672,14 +682,22 @@ public class OnEventWorker implements
ComponentClassTransformWorker2
}
private Object coerce(final String parameterName, Class
parameterType,
- String parameterValue, ValueEncoder
valueEncoder, boolean allowBlank)
+ String parameterValue, ValueEncoder
valueEncoder, boolean allowBlank, boolean treatBlankAsNull)
{
- if (!allowBlank && InternalUtils.isBlank(parameterValue))
+ if (InternalUtils.isBlank(parameterValue))
{
- throw new RuntimeException(String.format(
- "The value for query parameter '%s' was blank, but
a non-blank value is needed.",
- parameterName));
+ if (!allowBlank)
+ {
+ throw new RuntimeException(String.format(
+ "The value for query parameter '%s' was blank,
but a non-blank value is needed.",
+ parameterName));
+ }
+
+ if (treatBlankAsNull)
+ {
+ parameterValue = null;
+ }
}
Object value = valueEncoder.toValue(parameterValue);
diff --git a/tapestry-core/src/test/app1/RequestParameterDemo.tml
b/tapestry-core/src/test/app1/RequestParameterDemo.tml
index 683b9c4da..9c2fcc4c8 100644
--- a/tapestry-core/src/test/app1/RequestParameterDemo.tml
+++ b/tapestry-core/src/test/app1/RequestParameterDemo.tml
@@ -34,5 +34,11 @@
<li>
<a href="${blankAllowedLink}">Blank Allowed Link</a>
</li>
+ <li>
+ <a href="${blankAllowedLinkNonWrapperThrowsException}">Blank Allowed
Non-Wrapper Throws Exception Link</a>
+ </li>
+ <li>
+ <a href="${blankAllowedLinkNonWrapper}">Blank Allowed Non-Wrapper
Treat As Null Link</a>
+ </li>
</ul>
</html>
\ No newline at end of file
diff --git
a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/RequestParameterTests.java
b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/RequestParameterTests.java
index f2b845723..fbe2c186d 100644
---
a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/RequestParameterTests.java
+++
b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/RequestParameterTests.java
@@ -14,6 +14,8 @@ package org.apache.tapestry5.integration.app1;
import org.testng.annotations.Test;
+import spock.lang.Issue;
+
public class RequestParameterTests extends App1TestCase
{
@Test
@@ -74,4 +76,21 @@ public class RequestParameterTests extends App1TestCase
assertText("id=current", "-1");
}
+
+ @Issue("TAP5-2763")
+ @Test
+ public void
blank_allowed_for_non_wrapper_types_if_enabled_throws_exception() {
+ openLinks("RequestParameter Annotation Demo", "Blank Allowed
Non-Wrapper Throws Exception Link");
+
+ assertTextPresent("Coercion of to type java.time.LocalDateTime (via
String --> java.time.LocalDateTime) failed: Text '' could not be parsed at
index 0");
+ }
+
+ @Issue("TAP5-2763")
+ @Test
+ public void
blank_allowed_for_non_wrapper_types_if_enabled_and_treat_as_null() {
+ openLinks("RequestParameter Annotation Demo", "Blank Allowed
Non-Wrapper Treat As Null Link");
+
+ assertText("id=current", "-1");
+ }
+
}
diff --git
a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/RequestParameterDemo.java
b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/RequestParameterDemo.java
index 1556e86f6..e944ccf22 100644
---
a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/RequestParameterDemo.java
+++
b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/RequestParameterDemo.java
@@ -12,6 +12,11 @@
package org.apache.tapestry5.integration.app1.pages;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.util.Arrays;
+import java.util.List;
+
import org.apache.tapestry5.ComponentResources;
import org.apache.tapestry5.annotations.Persist;
import org.apache.tapestry5.annotations.Property;
@@ -19,9 +24,6 @@ import org.apache.tapestry5.annotations.RequestParameter;
import org.apache.tapestry5.http.Link;
import org.apache.tapestry5.ioc.annotations.Inject;
-import java.util.Arrays;
-import java.util.List;
-
public class RequestParameterDemo
{
private static final String PARAMETER_NAME = "gnip";
@@ -72,6 +74,16 @@ public class RequestParameterDemo
return
resources.createEventLink("emptyStringAllowed").addParameter(PARAMETER_NAME,
"");
}
+ public Link getBlankAllowedLinkNonWrapperThrowsException()
+ {
+ return
resources.createEventLink("emptyStringAllowedNonWrapperThrowsException").addParameter(PARAMETER_NAME,
"");
+ }
+
+ public Link getBlankAllowedLinkNonWrapper()
+ {
+ return
resources.createEventLink("emptyStringAllowedNonWrapper").addParameter(PARAMETER_NAME,
"");
+ }
+
public Link getNullLink()
{
return resources.createEventLink(EVENT_NAME);
@@ -111,4 +123,20 @@ public class RequestParameterDemo
}
}
+
+ void onEmptyStringAllowedNonWrapperThrowsException(@RequestParameter(value
= PARAMETER_NAME, allowBlank = true) LocalDateTime value)
+ {
+ this.value = -1;
+ }
+
+ void onEmptyStringAllowedNonWrapper(@RequestParameter(value =
PARAMETER_NAME, allowBlank = true, treatBlankAsNull = true) LocalDateTime value)
+ {
+ if (value == null)
+ {
+ this.value = -1;
+ } else
+ {
+ this.value = (int)
value.toInstant(ZoneOffset.UTC).getEpochSecond();
+ }
+ }
}
