You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by ff...@apache.org on 2021/07/21 15:29:06 UTC

[cxf] branch 3.4.x-fixes updated (f0c87b6 -> bb7754b)

This is an automated email from the ASF dual-hosted git repository.

ffang pushed a change to branch 3.4.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git.


    from f0c87b6  Update Dropwizard Metrics to 4.1.25
     new 817b10b  [CXF-8565] A failing test demonstrating the Netty Transport NPE A simple GET request with no body triggers an NPE because there is no content type for the request, and CXF tries to set a null Content-Type in the Netty Request Also add a test with a body, to ensure that Netty works reliably there as well
     new bb7754b  [CXF-8565] Add support for requests with no body to the netty client transport There were several issues with the netty client transport:

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../http/netty/client/NettyHttpConduit.java        | 62 +++++++++++++++++++---
 systests/jaxrs/pom.xml                             |  6 +++
 .../cxf/systest/jaxrs/JAXRSAsyncClientTest.java    | 47 ++++++++++++++++
 3 files changed, 107 insertions(+), 8 deletions(-)

[cxf] 01/02: [CXF-8565] A failing test demonstrating the Netty Transport NPE A simple GET request with no body triggers an NPE because there is no content type for the request, and CXF tries to set a null Content-Type in the Netty Request Also add a test with a body, to ensure that Netty works reliably there as well

Posted by ff...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ffang pushed a commit to branch 3.4.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 817b10b9089f29785e5f74f33e53a49f16dde320
Author: Tim Ward <ti...@apache.org>
AuthorDate: Wed Jul 21 13:52:22 2021 +0100

    [CXF-8565] A failing test demonstrating the Netty Transport NPE
    A simple GET request with no body triggers an NPE because there is no content type for the request, and CXF tries to set a null Content-Type in the Netty Request
    Also add a test with a body, to ensure that Netty works reliably there as well
    
    Signed-off-by: Tim Ward <ti...@apache.org>
    (cherry picked from commit e5f1c65f3bb2f0f043fb5572402a3932390b82c6)
---
 systests/jaxrs/pom.xml                             |  6 +++
 .../cxf/systest/jaxrs/JAXRSAsyncClientTest.java    | 47 ++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/systests/jaxrs/pom.xml b/systests/jaxrs/pom.xml
index 6208ffb..e686094 100644
--- a/systests/jaxrs/pom.xml
+++ b/systests/jaxrs/pom.xml
@@ -259,6 +259,12 @@
         </dependency>
         <dependency>
             <groupId>org.apache.cxf</groupId>
+            <artifactId>cxf-rt-transports-http-netty-client</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.cxf</groupId>
             <artifactId>cxf-rt-features-logging</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
index 3cbf337..15e4c04 100644
--- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
+++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
@@ -57,9 +57,11 @@ import javax.xml.ws.Holder;
 
 import org.apache.cxf.jaxrs.client.ClientConfiguration;
 import org.apache.cxf.jaxrs.client.JAXRSClientFactory;
+import org.apache.cxf.jaxrs.client.JAXRSClientFactoryBean;
 import org.apache.cxf.jaxrs.client.WebClient;
 import org.apache.cxf.jaxrs.model.AbstractResourceInfo;
 import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase;
+import org.apache.cxf.transport.http.netty.client.NettyHttpTransportFactory;
 import org.apache.cxf.transports.http.configuration.HTTPClientPolicy;
 
 import org.junit.Before;
@@ -478,6 +480,51 @@ public class JAXRSAsyncClientTest extends AbstractBusClientServerTestBase {
             assertThat(response.getStatus(), equalTo(404));
         }
     }
+
+    @Test
+    public void testNettyClientGet() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        
+        JAXRSClientFactoryBean bean = new JAXRSClientFactoryBean();
+        bean.setTransportId(NettyHttpTransportFactory.DEFAULT_NAMESPACES.get(0));
+        bean.setAddress(address);
+        
+        WebClient webClient = bean.createWebClient();
+        
+        try (Response response = webClient
+                .to(address, false)
+                .accept("text/plain")
+                .async()
+                .get()
+                .get(10, TimeUnit.SECONDS)) {
+            assertThat(response.getStatus(), equalTo(200));
+        } finally {
+            webClient.close();
+        }
+    }
+    
+    @Test
+    public void testNettyClientDeleteWithBody() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/deletebody";
+        
+        JAXRSClientFactoryBean bean = new JAXRSClientFactoryBean();
+        bean.setTransportId(NettyHttpTransportFactory.DEFAULT_NAMESPACES.get(0));
+        bean.setAddress(address);
+        
+        WebClient webClient = bean.createWebClient();
+        
+        try {
+            Book book = webClient
+                .to(address, false)
+                .accept("application/xml")
+                .async()
+                .method("DELETE", Entity.entity(new Book("Delete", 123L), "application/xml"), Book.class)
+                .get(20, TimeUnit.SECONDS);
+            assertEquals("Delete", book.getName());
+        } finally {
+            webClient.close();
+        }
+    }
     
     private WebClient createWebClient(String address) {
         return WebClient.create(address);

[cxf] 02/02: [CXF-8565] Add support for requests with no body to the netty client transport There were several issues with the netty client transport:

Posted by ff...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ffang pushed a commit to branch 3.4.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit bb7754baeb5e839061249ba32a48ba5f83bcb5e3
Author: Tim Ward <ti...@apache.org>
AuthorDate: Wed Jul 21 14:47:59 2021 +0100

    [CXF-8565] Add support for requests with no body to the netty client transport
    There were several issues with the netty client transport:
    
    * Several NPE locations due to setting or removing null headers
    * No call to connect when there is no OutputStream to write a body
    * No call to write the request when there is no OutputStream to close
    
    Signed-off-by: Tim Ward <ti...@apache.org>
    (cherry picked from commit fc5b8fd8689815ee3bff284b89ac8d9cb897e83b)
---
 .../http/netty/client/NettyHttpConduit.java        | 62 +++++++++++++++++++---
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java
index f5d4e7a..cd358d4 100644
--- a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java
+++ b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java
@@ -32,6 +32,8 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.Principal;
 import java.security.cert.Certificate;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -78,6 +80,9 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
     public static final String MAX_RESPONSE_CONTENT_LENGTH =
         "org.apache.cxf.transport.http.netty.maxResponseContentLength";
     static final Integer DEFAULT_MAX_RESPONSE_CONTENT_LENGTH = 1048576;
+    private static final Set<String> KNOWN_HTTP_VERBS_WITH_NO_CONTENT =
+            new HashSet<>(Arrays.asList(new String[]{"GET", "HEAD", "OPTIONS", "TRACE"}));
+    
     final NettyHttpConduitFactory factory;
     private Bootstrap bootstrap;
 
@@ -197,7 +202,10 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
             entity.createRequest(out.getOutBuffer());
             // TODO need to check how to set the Chunked feature
             //request.getRequest().setChunked(true);
-            entity.getRequest().headers().set(Message.CONTENT_TYPE, message.get(Message.CONTENT_TYPE));
+            Object contentType = message.get(Message.CONTENT_TYPE);
+            if (contentType != null) {
+                entity.getRequest().headers().set(Message.CONTENT_TYPE, contentType);
+            }
             return out;
         }
         return super.createOutputStream(message, needToCacheRequest, isChunking, chunkThreshold);
@@ -314,7 +322,7 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
                     
                     synchronized (entity) {
                         Channel syncChannel = getChannel();
-                        ChannelFuture channelFuture = syncChannel.write(entity);
+                        ChannelFuture channelFuture = syncChannel.writeAndFlush(entity);
                         channelFuture.addListener(listener);
                         outputStream.close();
                     }
@@ -328,6 +336,11 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
                 wrappedStream = cachedStream;
             }
         }
+        
+        @Override
+        protected void handleNoOutput() throws IOException {
+            connect(false);
+        }
 
         protected TLSClientParameters findTLSClientParameters() {
             TLSClientParameters clientParameters = outMessage.get(TLSClientParameters.class);
@@ -378,11 +391,6 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
 
             connFuture.addListener(listener);
 
-            if (!output) {
-                entity.getRequest().headers().remove("Transfer-Encoding");
-                entity.getRequest().headers().remove("Content-Type");
-                entity.getRequest().headers().remove(null);
-            }
 
             // setup the CxfResponseCallBack
             CxfResponseCallBack callBack = new CxfResponseCallBack() {
@@ -398,6 +406,29 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
             };
             entity.setCxfResponseCallback(callBack);
 
+            if (!output) {
+                entity.getRequest().headers().remove("Transfer-Encoding");
+                entity.getRequest().headers().remove("Content-Type");
+                
+                ChannelFutureListener writeFailureListener = new ChannelFutureListener() {
+                    @Override
+                    public void operationComplete(ChannelFuture future) throws Exception {
+                        if (!future.isSuccess()) {
+                            setException(future.cause());
+                        }
+                    }
+                };
+                
+                connFuture.addListener(new ChannelFutureListener() {
+                        @Override
+                        public void operationComplete(ChannelFuture future) throws Exception {
+                            if (future.isSuccess()) {
+                                ChannelFuture channelFuture = future.channel().writeAndFlush(entity);
+                                channelFuture.addListener(writeFailureListener);
+                            }
+                        }
+                    });
+            }
         }
 
         @Override
@@ -434,7 +465,7 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
         @Override
         protected void setProtocolHeaders() throws IOException {
             Headers h = new Headers(outMessage);
-            entity.getRequest().headers().set(Message.CONTENT_TYPE, h.determineContentType());
+            setContentTypeHeader(h);
             boolean addHeaders = MessageUtils.getContextualBoolean(outMessage, Headers.ADD_HEADERS_PROPERTY, false);
 
             for (Map.Entry<String, List<String>> header : h.headerMap().entrySet()) {
@@ -460,6 +491,21 @@ public class NettyHttpConduit extends URLConnectionHTTPConduit implements BusLif
                 }
             }
         }
+        
+        private void setContentTypeHeader(Headers headers) {
+            if (outMessage.get(Message.CONTENT_TYPE) == null) {
+                // if no content type is set then check for a request body
+                Object requestMethod = outMessage.get(Message.HTTP_REQUEST_METHOD);
+                boolean emptyRequest = KNOWN_HTTP_VERBS_WITH_NO_CONTENT.contains(requestMethod) 
+                        || PropertyUtils.isTrue(outMessage.get(Headers.EMPTY_REQUEST_PROPERTY));
+                // If it is not an empty request then add a content type
+                if (!emptyRequest) {
+                    entity.getRequest().headers().set(Message.CONTENT_TYPE, headers.determineContentType());
+                }
+            } else {
+                entity.getRequest().headers().set(Message.CONTENT_TYPE, headers.determineContentType());
+            }
+        }
 
         @Override
         protected void setFixedLengthStreamingMode(int i) {