You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2018/10/19 14:23:14 UTC

[cxf] 02/02: Fixing some issues thrown up by Spotbugs

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

coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit f68311f3de3e66526303b9292d07468499703eb6
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Oct 19 15:22:51 2018 +0100

    Fixing some issues thrown up by Spotbugs
---
 .../cxf/common/logging/RegexLoggingFilter.java     | 10 +++++-----
 .../cxf/transport/websocket/WebSocketUtils.java    |  6 ++++++
 .../AtmosphereWebSocketUndertowDestination.java    | 22 ++++++++++++++--------
 .../jetty9/Jetty9WebSocketDestination.java         | 15 ++++++++++-----
 .../undertow/UndertowWebSocketDestination.java     | 22 ++++++++++++++--------
 .../org/apache/cxf/sts/request/RequestParser.java  |  2 +-
 .../apache/cxf/tools/wsdlto/core/PluginLoader.java |  9 +++------
 7 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/core/src/main/java/org/apache/cxf/common/logging/RegexLoggingFilter.java b/core/src/main/java/org/apache/cxf/common/logging/RegexLoggingFilter.java
index 098b797..da89fbf 100644
--- a/core/src/main/java/org/apache/cxf/common/logging/RegexLoggingFilter.java
+++ b/core/src/main/java/org/apache/cxf/common/logging/RegexLoggingFilter.java
@@ -80,15 +80,15 @@ public class RegexLoggingFilter {
     }
 
     public void addCommandOption(String option, String... commands) {
-        String pattern = "(";
+        StringBuilder pattern = new StringBuilder("(");
         for (String command : commands) {
             if (pattern.length() > 1) {
-                pattern += "|";
+                pattern.append("|");
             }
-            pattern += Pattern.quote(command);
+            pattern.append(Pattern.quote(command));
         }
-        pattern += ") +.*?" + Pattern.quote(option) + " +([^ ]+)";
-        regexs.add(new ReplaceRegEx(pattern, 2, DEFAULT_REPLACEMENT));
+        pattern.append(") +.*?").append(Pattern.quote(option)).append(" +([^ ]+)");
+        regexs.add(new ReplaceRegEx(pattern.toString(), 2, DEFAULT_REPLACEMENT));
     }
 
     public String getPattern() {
diff --git a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/WebSocketUtils.java b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/WebSocketUtils.java
index 2633249..730e70d 100644
--- a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/WebSocketUtils.java
+++ b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/WebSocketUtils.java
@@ -25,6 +25,7 @@ import java.io.InputStream;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.TreeMap;
+import java.util.regex.Pattern;
 
 /**
  *
@@ -37,6 +38,7 @@ public final class WebSocketUtils {
 
     private static final byte[] CRLF = "\r\n".getBytes();
     private static final byte[] COLSP = ": ".getBytes();
+    private static final Pattern CR_OR_LF = Pattern.compile("\\r|\\n");
 
     private WebSocketUtils() {
     }
@@ -223,6 +225,10 @@ public final class WebSocketUtils {
         return sb.toByteArray();
     }
 
+    public static boolean isContainingCRLF(String value) {
+        return CR_OR_LF.matcher(value).find();
+    }
+
     private static class ByteArrayBuilder {
         private ByteArrayOutputStream baos;
         ByteArrayBuilder() {
diff --git a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/atmosphere/AtmosphereWebSocketUndertowDestination.java b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/atmosphere/AtmosphereWebSocketUndertowDestination.java
index d894482..dafb898 100644
--- a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/atmosphere/AtmosphereWebSocketUndertowDestination.java
+++ b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/atmosphere/AtmosphereWebSocketUndertowDestination.java
@@ -45,6 +45,7 @@ import org.apache.cxf.transport.http_undertow.UndertowHTTPHandler;
 import org.apache.cxf.transport.http_undertow.UndertowHTTPServerEngineFactory;
 import org.apache.cxf.transport.websocket.WebSocketConstants;
 import org.apache.cxf.transport.websocket.WebSocketDestinationService;
+import org.apache.cxf.transport.websocket.WebSocketUtils;
 import org.apache.cxf.transport.websocket.undertow.WebSocketUndertowServletRequest;
 import org.apache.cxf.transport.websocket.undertow.WebSocketUndertowServletResponse;
 import org.apache.cxf.workqueue.WorkQueueManager;
@@ -186,7 +187,7 @@ public class AtmosphereWebSocketUndertowDestination extends UndertowHTTPDestinat
                     public void handleUpgrade(StreamConnection streamConnection,
                                               HttpServerExchange exchange) {
                         try {
-                            
+
                             WebSocketChannel channel = selected.createChannel(facade, streamConnection,
                                                                               facade.getBufferPool());
                             peerConnections.add(channel);
@@ -226,7 +227,7 @@ public class AtmosphereWebSocketUndertowDestination extends UndertowHTTPDestinat
                 .getDeployment(), request, response, null);
 
             undertowExchange.putAttachment(ServletRequestContext.ATTACHMENT_KEY, servletRequestContext);
-            
+
             try {
                 framework.doCometSupport(AtmosphereRequestImpl.wrap(request),
                                          AtmosphereResponseImpl.wrap(response));
@@ -246,7 +247,7 @@ public class AtmosphereWebSocketUndertowDestination extends UndertowHTTPDestinat
             } catch (ServletException e) {
                 throw new IOException(e);
             }
-            
+
         }
 
         private void handleReceivedMessage(WebSocketChannel channel, Object message, HttpServerExchange exchange) {
@@ -258,18 +259,23 @@ public class AtmosphereWebSocketUndertowDestination extends UndertowHTTPDestinat
                         HttpServletRequest request = new WebSocketUndertowServletRequest(channel, message, exchange);
                         HttpServletResponse response = new WebSocketUndertowServletResponse(channel);
                         if (request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY) != null) {
-                            response.setHeader(WebSocketConstants.DEFAULT_RESPONSE_ID_KEY,
-                                               request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY));
+                            String headerValue = request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY);
+                            if (WebSocketUtils.isContainingCRLF(headerValue)) {
+                                LOG.warning("Invalid characters (CR/LF) in header "
+                                    + WebSocketConstants.DEFAULT_REQUEST_ID_KEY);
+                            } else {
+                                response.setHeader(WebSocketConstants.DEFAULT_RESPONSE_ID_KEY, headerValue);
+                            }
                         }
                         handleNormalRequest(request, response);
                     } catch (Exception ex) {
                         LOG.log(Level.WARNING, "Failed to invoke service", ex);
                     }
-                    
+
                 }
-                
+
             });
-            
+
         }
     }
 
diff --git a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/jetty9/Jetty9WebSocketDestination.java b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/jetty9/Jetty9WebSocketDestination.java
index b134efb..ba9fcf2 100644
--- a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/jetty9/Jetty9WebSocketDestination.java
+++ b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/jetty9/Jetty9WebSocketDestination.java
@@ -49,6 +49,7 @@ import org.apache.cxf.transport.http_jetty.JettyHTTPServerEngineFactory;
 import org.apache.cxf.transport.websocket.InvalidPathException;
 import org.apache.cxf.transport.websocket.WebSocketConstants;
 import org.apache.cxf.transport.websocket.WebSocketDestinationService;
+import org.apache.cxf.transport.websocket.WebSocketUtils;
 import org.apache.cxf.transport.websocket.jetty.WebSocketServletHolder;
 import org.apache.cxf.transport.websocket.jetty.WebSocketVirtualServletRequest;
 import org.apache.cxf.transport.websocket.jetty.WebSocketVirtualServletResponse;
@@ -97,9 +98,9 @@ public class Jetty9WebSocketDestination extends JettyHTTPDestination implements
                        final ServletContext context,
                        final HttpServletRequest request,
                        final HttpServletResponse response) throws IOException {
-        
+
         WebSocketServletFactory wsf = getWebSocketFactory(config, context);
-       
+
         if (wsf.isUpgradeRequest(request, response)
             && wsf.acceptWebSocket(request, response)) {
             ((Request)request).setHandled(true);
@@ -119,13 +120,13 @@ public class Jetty9WebSocketDestination extends JettyHTTPDestination implements
     protected String getAddress(EndpointInfo endpointInfo) {
         return getNonWSAddress(endpointInfo);
     }
-    
+
     Server getServer(ServletConfig config, ServletContext context) {
         ContextHandler.Context c = (ContextHandler.Context)context;
         ContextHandler h = c.getContextHandler();
         return h.getServer();
     }
-    
+
     private WebSocketServletFactory getWebSocketFactory(ServletConfig config, ServletContext context) {
         if (webSocketFactory == null) {
             Server server = getServer(config, context);
@@ -184,7 +185,11 @@ public class Jetty9WebSocketDestination extends JettyHTTPDestination implements
                     request = createServletRequest(data, offset, length, holder, session);
                     String reqid = request.getHeader(REQUEST_ID_KEY);
                     if (reqid != null) {
-                        response.setHeader(RESPONSE_ID_KEY, reqid);
+                        if (WebSocketUtils.isContainingCRLF(reqid)) {
+                            LOG.warning("Invalid characters (CR/LF) in header " + REQUEST_ID_KEY);
+                        } else {
+                            response.setHeader(RESPONSE_ID_KEY, reqid);
+                        }
                     }
                     invoke(null, null, request, response);
                 } catch (InvalidPathException ex) {
diff --git a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/undertow/UndertowWebSocketDestination.java b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/undertow/UndertowWebSocketDestination.java
index 1d34169..78233c8 100644
--- a/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/undertow/UndertowWebSocketDestination.java
+++ b/rt/transports/websocket/src/main/java/org/apache/cxf/transport/websocket/undertow/UndertowWebSocketDestination.java
@@ -44,6 +44,7 @@ import org.apache.cxf.transport.http_undertow.UndertowHTTPHandler;
 import org.apache.cxf.transport.http_undertow.UndertowHTTPServerEngineFactory;
 import org.apache.cxf.transport.websocket.WebSocketConstants;
 import org.apache.cxf.transport.websocket.WebSocketDestinationService;
+import org.apache.cxf.transport.websocket.WebSocketUtils;
 import org.apache.cxf.workqueue.WorkQueueManager;
 import org.xnio.StreamConnection;
 
@@ -71,7 +72,7 @@ public class UndertowWebSocketDestination extends UndertowHTTPDestination
     implements WebSocketDestinationService {
     private static final Logger LOG = LogUtils.getL7dLogger(UndertowWebSocketDestination.class);
     private final Executor executor;
-        
+
     public UndertowWebSocketDestination(Bus bus, DestinationRegistry registry, EndpointInfo ei,
                                                   UndertowHTTPServerEngineFactory serverEngineFactory)
                                                       throws IOException {
@@ -156,7 +157,7 @@ public class UndertowWebSocketDestination extends UndertowHTTPDestination
                     public void handleUpgrade(StreamConnection streamConnection,
                                               HttpServerExchange exchange) {
                         try {
-                            
+
                             WebSocketChannel channel = selected.createChannel(facade, streamConnection,
                                                                               facade.getBufferPool());
                             peerConnections.add(channel);
@@ -177,7 +178,7 @@ public class UndertowWebSocketDestination extends UndertowHTTPDestination
                                 }
                             });
                             channel.resumeReceives();
-                            
+
                         } catch (Exception e) {
                             LOG.log(Level.WARNING, "Failed to invoke service", e);
                         }
@@ -215,18 +216,23 @@ public class UndertowWebSocketDestination extends UndertowHTTPDestination
                         HttpServletRequest request = new WebSocketUndertowServletRequest(channel, message, exchange);
                         HttpServletResponse response = new WebSocketUndertowServletResponse(channel);
                         if (request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY) != null) {
-                            response.setHeader(WebSocketConstants.DEFAULT_RESPONSE_ID_KEY,
-                                               request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY));
+                            String headerValue = request.getHeader(WebSocketConstants.DEFAULT_REQUEST_ID_KEY);
+                            if (WebSocketUtils.isContainingCRLF(headerValue)) {
+                                LOG.warning("Invalid characters (CR/LF) in header "
+                                    + WebSocketConstants.DEFAULT_REQUEST_ID_KEY);
+                            } else {
+                                response.setHeader(WebSocketConstants.DEFAULT_RESPONSE_ID_KEY, headerValue);
+                            }
                         }
                         handleNormalRequest(request, response);
                     } catch (Exception ex) {
                         ex.printStackTrace();
                     }
-                    
+
                 }
-                
+
             });
-            
+
         }
     }
 }
diff --git a/services/sts/sts-core/src/main/java/org/apache/cxf/sts/request/RequestParser.java b/services/sts/sts-core/src/main/java/org/apache/cxf/sts/request/RequestParser.java
index 1ba65ab..befdbf5 100644
--- a/services/sts/sts-core/src/main/java/org/apache/cxf/sts/request/RequestParser.java
+++ b/services/sts/sts-core/src/main/java/org/apache/cxf/sts/request/RequestParser.java
@@ -111,7 +111,7 @@ public class RequestParser {
             // JAXB types
             if (requestObject instanceof JAXBElement<?>) {
                 JAXBElement<?> jaxbElement = (JAXBElement<?>) requestObject;
-                if (jaxbElement != null && LOG.isLoggable(Level.FINE)) {
+                if (LOG.isLoggable(Level.FINE)) {
                     LOG.fine("Found " + jaxbElement.getName() + ": " + jaxbElement.getValue());
                 }
                 try {
diff --git a/tools/wsdlto/core/src/main/java/org/apache/cxf/tools/wsdlto/core/PluginLoader.java b/tools/wsdlto/core/src/main/java/org/apache/cxf/tools/wsdlto/core/PluginLoader.java
index 3d83608..dc3f6a1 100644
--- a/tools/wsdlto/core/src/main/java/org/apache/cxf/tools/wsdlto/core/PluginLoader.java
+++ b/tools/wsdlto/core/src/main/java/org/apache/cxf/tools/wsdlto/core/PluginLoader.java
@@ -188,10 +188,10 @@ public final class PluginLoader {
 
     protected Plugin getPlugin(URL url) throws IOException, JAXBException, FileNotFoundException {
         Plugin plugin = plugins.get(url.toString());
-        InputStream is = null;
         if (plugin == null) {
-            is = url.openStream();
-            plugin = getPlugin(is);
+            try (InputStream is = url.openStream()) {
+                plugin = getPlugin(is);
+            }
             if (plugin == null || StringUtils.isEmpty(plugin.getName())) {
                 Message msg = new Message("PLUGIN_LOAD_FAIL", LOG, url);
                 LOG.log(Level.SEVERE, msg.toString());
@@ -199,9 +199,6 @@ public final class PluginLoader {
             }
             plugins.put(url.toString(), plugin);
         }
-        if (is == null) {
-            return getPlugin(url.toString());
-        }
         return plugin;
     }