From b83c9f42ea06ebd9b27bdfcc7166cffbd235330e Mon Sep 17 00:00:00 2001 From: Wadeck Follonier Date: Tue, 27 Aug 2024 14:00:29 +0200 Subject: [PATCH 1/2] [JENKINS-73690] Avoid null Enum to throw an exception It's expected to be supported as there is an option to make the QueryParameter required. Positive test cases added as well. --- .../java/org/kohsuke/stapler/Stapler.java | 3 + .../org/kohsuke/stapler/DataBindingTest.java | 89 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/core/src/main/java/org/kohsuke/stapler/Stapler.java b/core/src/main/java/org/kohsuke/stapler/Stapler.java index 5cd869a1c..8e3a769b5 100644 --- a/core/src/main/java/org/kohsuke/stapler/Stapler.java +++ b/core/src/main/java/org/kohsuke/stapler/Stapler.java @@ -1255,6 +1255,9 @@ public org.apache.commons.fileupload.FileItem convert(Class type, Object value) private static final Converter ENUM_CONVERTER = new Converter() { @Override public Object convert(Class type, Object value) { + if (value == null) { + return null; + } return Enum.valueOf(type, value.toString()); } }; diff --git a/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java b/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java index 2f44fa16e..640833803 100644 --- a/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java +++ b/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java @@ -11,9 +11,11 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; +import javax.servlet.ServletException; import junit.framework.TestCase; import net.sf.json.JSONArray; import net.sf.json.JSONObject; +import org.jvnet.hudson.test.Issue; /** * @author Kohsuke Kawaguchi @@ -430,4 +432,91 @@ public void testDerivedProperty() { DerivedProperty r = bind("{items:[1,3,5]}", DerivedProperty.class); assertEquals(Arrays.asList(1, 3, 5), r.getItems()); } + + @Issue("JENKINS-73690") + public void testEnumCanBeNull() throws Exception { + MockRequest mr = new MockRequest() { + @Override + public String getContentType() { + return "text/html"; + } + }; + mr.getParameterMap().put("status", null); + RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); + Object result = new Function.InstanceFunction( + getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + .bindAndInvoke(this, req, null); + assertNull(result); + } + + public void testEnumIsSupported() throws Exception { + MockRequest mr = new MockRequest() { + @Override + public String getContentType() { + return "text/html"; + } + }; + mr.getParameterMap().put("status", "Done"); + RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); + Object result = new Function.InstanceFunction( + getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + .bindAndInvoke(this, req, null); + assertEquals(Status.Done, result); + } + + public void testEnumUnknownIsThrowing() throws Exception { + MockRequest mr = new MockRequest() { + @Override + public String getContentType() { + return "text/html"; + } + }; + mr.getParameterMap().put("status", "Blabla"); + RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); + try { + new Function.InstanceFunction( + getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + .bindAndInvoke(this, req, null); + fail("Should throw an exception as the enum value is not recognized"); + } + catch (IllegalArgumentException e){ + assertEquals("No enum constant org.kohsuke.stapler.DataBindingTest.Status.Blabla", e.getCause().getMessage()); + } + } + + // Just to ensure to keep support for required=true + @Issue("JENKINS-73690") + public void testEnumCannotBeNullIfRequired() throws Exception { + MockRequest mr = new MockRequest() { + @Override + public String getContentType() { + return "text/html"; + } + }; + mr.getParameterMap().put("status", null); + RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); + try { + new Function.InstanceFunction( + getClass().getMethod("doRequireEnum", StaplerRequest.class, Status.class)) + .bindAndInvoke(this, req, null); + fail("Should throw an exception as the enum value is not recognized"); + } + catch (ServletException e){ + assertEquals("Required Query parameter status is missing", e.getMessage()); + } + } + + enum Status { + Todo, + Progress, + Done + } + + public Object doAcceptEnum(StaplerRequest req, @QueryParameter Status status) { + return status; + } + + public Object doRequireEnum(StaplerRequest req, @QueryParameter(required = true) Status status) { + return status; + } } From 5bf1407d09a0564b62b25f57c9ba392c961429dd Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Tue, 27 Aug 2024 06:39:06 -0600 Subject: [PATCH 2/2] Apply spotless formatting --- .../org/kohsuke/stapler/DataBindingTest.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java b/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java index 640833803..444c48226 100644 --- a/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java +++ b/core/src/test/java/org/kohsuke/stapler/DataBindingTest.java @@ -444,7 +444,7 @@ public String getContentType() { mr.getParameterMap().put("status", null); RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); Object result = new Function.InstanceFunction( - getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) .bindAndInvoke(this, req, null); assertNull(result); } @@ -459,7 +459,7 @@ public String getContentType() { mr.getParameterMap().put("status", "Done"); RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); Object result = new Function.InstanceFunction( - getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) .bindAndInvoke(this, req, null); assertEquals(Status.Done, result); } @@ -474,13 +474,13 @@ public String getContentType() { mr.getParameterMap().put("status", "Blabla"); RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); try { - new Function.InstanceFunction( - getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) + new Function.InstanceFunction(getClass().getMethod("doAcceptEnum", StaplerRequest.class, Status.class)) .bindAndInvoke(this, req, null); fail("Should throw an exception as the enum value is not recognized"); - } - catch (IllegalArgumentException e){ - assertEquals("No enum constant org.kohsuke.stapler.DataBindingTest.Status.Blabla", e.getCause().getMessage()); + } catch (IllegalArgumentException e) { + assertEquals( + "No enum constant org.kohsuke.stapler.DataBindingTest.Status.Blabla", + e.getCause().getMessage()); } } @@ -496,12 +496,10 @@ public String getContentType() { mr.getParameterMap().put("status", null); RequestImpl req = new RequestImpl(new Stapler(), mr, Collections.emptyList(), null); try { - new Function.InstanceFunction( - getClass().getMethod("doRequireEnum", StaplerRequest.class, Status.class)) + new Function.InstanceFunction(getClass().getMethod("doRequireEnum", StaplerRequest.class, Status.class)) .bindAndInvoke(this, req, null); fail("Should throw an exception as the enum value is not recognized"); - } - catch (ServletException e){ + } catch (ServletException e) { assertEquals("Required Query parameter status is missing", e.getMessage()); } }