[ 
https://issues.apache.org/jira/browse/CXF-7860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16642641#comment-16642641
 ] 

ASF GitHub Bot commented on CXF-7860:
-------------------------------------

andymc12 closed pull request #453: CXF-7860: Ensure @FormParam parms are 
consistent with Form entities
URL: https://github.com/apache/cxf/pull/453
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java 
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
index 4685270e5e1..90276785327 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
@@ -769,49 +769,68 @@ public static float getMediaTypeQualityFactor(String q) {
         boolean preferModelParams = paramsInfo.size() > parameterTypes.length
             && 
!PropertyUtils.isTrue(message.getContextualProperty("org.apache.cxf.preferMethodParameters"));
 
-        int parameterTypesLengh = preferModelParams ? paramsInfo.size() : 
parameterTypes.length;
+        final int parameterTypesLength = preferModelParams ? paramsInfo.size() 
: parameterTypes.length;
+        if (parameterTypesLength < 1) {
+            return Collections.emptyList();
+        }
 
         Type[] genericParameterTypes = ori.getInGenericParameterTypes();
         Annotation[][] anns = ori.getInParameterAnnotations();
-        List<Object> params = new ArrayList<>(parameterTypesLengh);
+        Object[] params = new Object[parameterTypesLength];
 
-        for (int i = 0; i < parameterTypesLengh; i++) {
-            Class<?> param = null;
-            Type genericParam = null;
-            Annotation[] paramAnns = null;
+        // Ensure we process all request-body parameters first, then all 
@*Params, etc.
+        ParamTuple[] tuple = new ParamTuple[parameterTypesLength];
+        for (int i = 0; i < parameterTypesLength; i++) {
+            tuple[i] = new ParamTuple();
             if (!preferModelParams) {
-                param = parameterTypes[i];
-                genericParam = InjectionUtils.processGenericTypeIfNeeded(
-                    ori.getClassResourceInfo().getServiceClass(), param, 
genericParameterTypes[i]);
-                param = InjectionUtils.updateParamClassToTypeIfNeeded(param, 
genericParam);
-                paramAnns = anns == null ? EMPTY_ANNOTATIONS : anns[i];
+                tuple[i].param = parameterTypes[i];
+                tuple[i].genericParam = 
InjectionUtils.processGenericTypeIfNeeded(
+                    ori.getClassResourceInfo().getServiceClass(), 
tuple[i].param, genericParameterTypes[i]);
+                tuple[i].param = 
InjectionUtils.updateParamClassToTypeIfNeeded(tuple[i].param, 
+                                                                               
tuple[i].genericParam);
+                tuple[i].paramAnns = anns == null ? EMPTY_ANNOTATIONS : 
anns[i];
             } else {
-                param = paramsInfo.get(i).getJavaType();
-                genericParam = param;
-                paramAnns = EMPTY_ANNOTATIONS;
+                tuple[i].param = paramsInfo.get(i).getJavaType();
+                tuple[i].genericParam = tuple[i].param;
+                tuple[i].paramAnns = EMPTY_ANNOTATIONS;
+            }
+            if (paramsInfo.get(i).getType() == ParameterType.REQUEST_BODY) {
+                params[i] = processRequestBodyParameter(tuple[i].param, 
+                                                        tuple[i].genericParam, 
+                                                        tuple[i].paramAnns,
+                                                        message,
+                                                        ori);
             }
+        }
+        for (int i = 0; i < parameterTypesLength; i++) {
 
-            Object paramValue = processParameter(param,
-                                                 genericParam,
-                                                 paramAnns,
-                                                 paramsInfo.get(i),
-                                                 values,
-                                                 message,
-                                                 ori);
-            params.add(paramValue);
+            if (paramsInfo.get(i).getType() != ParameterType.REQUEST_BODY) {
+                params[i] = processParameter(tuple[i].param,
+                                             tuple[i].genericParam,
+                                             tuple[i].paramAnns,
+                                             paramsInfo.get(i),
+                                             values,
+                                             message,
+                                             ori);
+            }
         }
 
-        return params;
+        return Arrays.asList(params);
     }
 
-    private static Object processParameter(Class<?> parameterClass,
-                                           Type parameterType,
-                                           Annotation[] parameterAnns,
-                                           Parameter parameter,
-                                           MultivaluedMap<String, String> 
values,
-                                           Message message,
-                                           OperationResourceInfo ori)
+    private static class ParamTuple {
+        private Class<?> param;
+        private Type genericParam;
+        private Annotation[] paramAnns;
+    }
+
+    private static Object processRequestBodyParameter(Class<?> parameterClass,
+                                                      Type parameterType,
+                                                      Annotation[] 
parameterAnns,
+                                                      Message message,
+                                                      OperationResourceInfo 
ori)
         throws IOException, WebApplicationException {
+
         InputStream is = message.getContent(InputStream.class);
         if (is == null) {
             Reader reader = message.getContent(Reader.class);
@@ -819,26 +838,38 @@ private static Object processParameter(Class<?> 
parameterClass,
                 is = new ReaderInputStream(reader);
             }
         }
-        if (parameter.getType() == ParameterType.REQUEST_BODY) {
 
-            if (parameterClass == AsyncResponse.class) {
-                return new AsyncResponseImpl(message);
-            }
+        if (parameterClass == AsyncResponse.class) {
+            return new AsyncResponseImpl(message);
+        }
 
-            String contentType = (String)message.get(Message.CONTENT_TYPE);
+        String contentType = (String)message.get(Message.CONTENT_TYPE);
 
-            if (contentType == null) {
-                String defaultCt = 
(String)message.getContextualProperty(DEFAULT_CONTENT_TYPE);
-                contentType = defaultCt == null ? 
MediaType.APPLICATION_OCTET_STREAM : defaultCt;
-            }
+        if (contentType == null) {
+            String defaultCt = 
(String)message.getContextualProperty(DEFAULT_CONTENT_TYPE);
+            contentType = defaultCt == null ? 
MediaType.APPLICATION_OCTET_STREAM : defaultCt;
+        }
+
+        return readFromMessageBody(parameterClass,
+                                   parameterType,
+                                   parameterAnns,
+                                   is,
+                                   toMediaType(contentType),
+                                   ori,
+                                   message);
+    }
 
-            return readFromMessageBody(parameterClass,
-                                       parameterType,
-                                       parameterAnns,
-                                       is,
-                                       toMediaType(contentType),
-                                       ori,
-                                       message);
+    private static Object processParameter(Class<?> parameterClass,
+                                           Type parameterType,
+                                           Annotation[] parameterAnns,
+                                           Parameter parameter,
+                                           MultivaluedMap<String, String> 
values,
+                                           Message message,
+                                           OperationResourceInfo ori)
+        throws IOException, WebApplicationException {
+
+        if (parameter.getType() == ParameterType.REQUEST_BODY) {
+            return processRequestBodyParameter(parameterClass, parameterType, 
parameterAnns, message, ori);
         } else if (parameter.getType() == ParameterType.CONTEXT) {
             return createContextValue(message, parameterType, parameterClass);
         } else if (parameter.getType() == ParameterType.BEAN) {
diff --git 
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormBehaviorTest.java
 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormBehaviorTest.java
new file mode 100644
index 00000000000..145967f2a13
--- /dev/null
+++ 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormBehaviorTest.java
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.systest.jaxrs.form;
+
+import javax.ws.rs.client.Client;
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Entity;
+import javax.ws.rs.core.Form;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+import org.apache.cxf.jaxrs.JAXRSServerFactoryBean;
+import org.apache.cxf.jaxrs.model.AbstractResourceInfo;
+import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase;
+import org.apache.cxf.testutil.common.AbstractBusTestServerBase;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class FormBehaviorTest extends AbstractBusClientServerTestBase {
+    public static final String PORT = allocatePort(FormBehaviorTest.class);
+
+    @Ignore
+    public static class Server extends AbstractBusTestServerBase {
+        protected void run() {
+            final JAXRSServerFactoryBean sf = new JAXRSServerFactoryBean();
+            sf.setResourceClasses(FormResource.class);
+            sf.setProvider(new FormReaderInterceptor());
+            sf.setAddress("http://localhost:"; + PORT + "/");
+            sf.create();
+        }
+
+        public static void main(String[] args) {
+            try {
+                Server s = new Server();
+                s.start();
+            } catch (Exception ex) {
+                ex.printStackTrace();
+                System.exit(-1);
+            } finally {
+                System.out.println("done!");
+            }
+        }
+    }
+
+    @BeforeClass
+    public static void startServers() throws Exception {
+        AbstractResourceInfo.clearAllMaps();
+        // keep out of process due to stack traces testing failures
+        assertTrue("server did not launch correctly", 
launchServer(Server.class, true));
+        createStaticBus();
+    }
+
+    @Test
+    public void testInterceptorInvokedOnFormAndFormParamMatchesFormValue() 
throws Exception {
+        Client client = ClientBuilder.newClient();
+        String uri = "http://localhost:"; + PORT + "/form";
+        Form f = new Form("value", "ORIGINAL");
+        Response r = client.target(uri)
+                           .request(MediaType.APPLICATION_FORM_URLENCODED)
+                           .post(Entity.form(f));
+
+        Assert.assertEquals("MODIFIED", r.getHeaderString("FromForm"));
+        Assert.assertEquals("MODIFIED", r.getHeaderString("FromFormParam"));
+    }
+}
diff --git 
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormReaderInterceptor.java
 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormReaderInterceptor.java
new file mode 100644
index 00000000000..a81ad288092
--- /dev/null
+++ 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormReaderInterceptor.java
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.systest.jaxrs.form;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.logging.Logger;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.ext.Provider;
+import javax.ws.rs.ext.ReaderInterceptor;
+import javax.ws.rs.ext.ReaderInterceptorContext;
+
+import org.apache.cxf.common.logging.LogUtils;
+
+@Provider
+public class FormReaderInterceptor implements ReaderInterceptor {
+    private static final Logger LOG = 
LogUtils.getL7dLogger(FormReaderInterceptor.class);
+
+    @Override
+    public Object aroundReadFrom(ReaderInterceptorContext ctx) throws 
IOException, WebApplicationException {
+        BufferedReader br = new BufferedReader(new 
InputStreamReader(ctx.getInputStream()));
+        String line;
+        while ((line = br.readLine()) != null) {
+            LOG.info("readLine: " + line);
+        }
+
+        ByteArrayInputStream bais = new 
ByteArrayInputStream("value=MODIFIED".getBytes());
+        LOG.info("set value=MODIFIED");
+        ctx.setInputStream(bais);
+        return ctx.proceed();
+    }
+
+}
diff --git 
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormResource.java
 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormResource.java
new file mode 100644
index 00000000000..b1816d383e4
--- /dev/null
+++ 
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/form/FormResource.java
@@ -0,0 +1,45 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.systest.jaxrs.form;
+
+import java.util.logging.Logger;
+
+import javax.ws.rs.FormParam;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.core.Form;
+import javax.ws.rs.core.Response;
+
+import org.apache.cxf.common.logging.LogUtils;
+
+@Path("/form")
+public class FormResource {
+    private static final Logger LOG = 
LogUtils.getL7dLogger(FormResource.class);
+
+    @POST
+    public Response processForm(@FormParam("value") String value, Form form) {
+        String fromForm = form.asMap().getFirst("value");
+        LOG.info("FromFormParam: " + value);
+        LOG.info("FromForm: " + fromForm);
+        return Response.ok()
+                        .header("FromFormParam", value)
+                        .header("FromForm", fromForm)
+                        .build();
+    }
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> JAX-RS @FormParam parameters are not updated when form content is modified
> --------------------------------------------------------------------------
>
>                 Key: CXF-7860
>                 URL: https://issues.apache.org/jira/browse/CXF-7860
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 3.2.6
>            Reporter: Andy McCright
>            Assignee: Andy McCright
>            Priority: Major
>
> The JAX-RS community noticed a difference in behavior between CXF and 
> Jersey/RESTEasy where form data in the HTTP body is modified prior to 
> invoking the resource method.  There are two differences noted:
> 1) CXF does not invoke a MessageBodyReader (or any ReaderInterceptors) when 
> there is no entity parameter.  I believe that this is proper behavior - or at 
> least an appropriate optimization since there is no parameter, why bother 
> creating one with a MBR (and it's associated ReaderInterceptors)?  
> 2) When the resource method contains both a Form parameter (entity) _and_ a 
> parameter annotated with `@FormParam`, and a Filter or interceptor, etc. has 
> modified the content of of the HTTP body, the value injected for the 
> `@FormParam` parameter does not reflect those modifications, but the Form 
> entity parameter does.  This seems inconsistent, and (IMO) violates the 
> spirit of the spec - note that there is no TCK test for this case, so CXF is 
> still compliant - but it differs from other implementations.  Here is an 
> example:
>  
>  
> {code:java}
> @Provider
> public class FormReaderInterceptor implements ReaderInterceptor {
>     private static final Logger LOG = 
> LogUtils.getL7dLogger(FormReaderInterceptor.class);
>     @Override
>     public Object aroundReadFrom(ReaderInterceptorContext ctx) throws 
> IOException, WebApplicationException {
>         BufferedReader br = new BufferedReader(new 
> InputStreamReader(ctx.getInputStream()));
>         String line;
>         while ((line = br.readLine()) != null) {
>             LOG.info("readLine: " + line);
>         }
>         ByteArrayInputStream bais = new 
> ByteArrayInputStream("value=MODIFIED".getBytes());
>         ctx.setInputStream(bais);
>         return ctx.proceed();
>     }
> }
> {code}
>  
> {code:java}
>     @POST
>     public Response processForm(@FormParam("value") String value, Form form) 
> {...
>  {code}
>  
> If the HTTP request body contains "value=ORIGINAL", then when CXF invokes the 
> processForm method, it will pass "ORIGINAL" to the String value, but will 
> pass a Form object that contains a MultivaluedMap with entry 
> "value=MODIFIED".  To be consistent with Jersey and RESTEasy, CXF should 
> inject MODIFIED as the String value.
>  
> See JAX-RS API [Issue 
> 659|https://github.com/eclipse-ee4j/jaxrs-api/issues/659] for more details.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to