You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/05/22 20:50:34 UTC

[tomcat] branch master updated: Code style

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

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


The following commit(s) were added to refs/heads/master by this push:
     new cbe9c72  Code style
cbe9c72 is described below

commit cbe9c72d78ddf450d19e7ffe846cdc328c337b0b
Author: remm <re...@apache.org>
AuthorDate: Wed May 22 22:50:22 2019 +0200

    Code style
    
    There's a lot of code in common in SecureNioXChannel, so cleanup before
    looking at it.
---
 .../apache/tomcat/util/net/SecureNio2Channel.java  |   8 +-
 .../apache/tomcat/util/net/SecureNioChannel.java   | 142 +++++++++++++--------
 2 files changed, 95 insertions(+), 55 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/SecureNio2Channel.java b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
index 61ed253..a45c9a5 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -55,22 +55,22 @@ public class SecureNio2Channel extends Nio2Channel  {
     // various scenarios
     private static final int DEFAULT_NET_BUFFER_SIZE = 16921;
 
+    protected final Nio2Endpoint endpoint;
+
     protected ByteBuffer netInBuffer;
     protected ByteBuffer netOutBuffer;
 
     protected SSLEngine sslEngine;
-    protected final Nio2Endpoint endpoint;
 
     protected boolean sniComplete = false;
 
-    private volatile boolean handshakeComplete;
+    private volatile boolean handshakeComplete = false;
     private volatile HandshakeStatus handshakeStatus; //gets set by handshake
 
-    private volatile boolean unwrapBeforeRead = false;
-
     protected boolean closed;
     protected boolean closing;
 
+    private volatile boolean unwrapBeforeRead = false;
     private final CompletionHandler<Integer, SocketWrapperBase<Nio2Channel>> handshakeReadCompletionHandler;
     private final CompletionHandler<Integer, SocketWrapperBase<Nio2Channel>> handshakeWriteCompletionHandler;
 
diff --git a/java/org/apache/tomcat/util/net/SecureNioChannel.java b/java/org/apache/tomcat/util/net/SecureNioChannel.java
index 7458b21..6f32cf3 100644
--- a/java/org/apache/tomcat/util/net/SecureNioChannel.java
+++ b/java/org/apache/tomcat/util/net/SecureNioChannel.java
@@ -52,6 +52,8 @@ public class SecureNioChannel extends NioChannel {
     // various scenarios
     private static final int DEFAULT_NET_BUFFER_SIZE = 16921;
 
+    private final NioEndpoint endpoint;
+
     protected ByteBuffer netInBuffer;
     protected ByteBuffer netOutBuffer;
 
@@ -66,7 +68,6 @@ public class SecureNioChannel extends NioChannel {
     protected boolean closing = false;
 
     protected NioSelectorPool pool;
-    private final NioEndpoint endpoint;
 
     public SecureNioChannel(SocketChannel channel, SocketBufferHandler bufHandler,
             NioSelectorPool pool, NioEndpoint endpoint) {
@@ -140,10 +141,9 @@ public class SecureNioChannel extends NioChannel {
      */
     protected boolean flush(ByteBuffer buf) throws IOException {
         int remaining = buf.remaining();
-        if ( remaining > 0 ) {
-            int written = sc.write(buf);
-            return written >= remaining;
-        }else {
+        if (remaining > 0) {
+            return (sc.write(buf) >= remaining);
+        } else {
             return true;
         }
     }
@@ -180,17 +180,18 @@ public class SecureNioChannel extends NioChannel {
             }
         }
 
-        if (!flush(netOutBuffer)) return SelectionKey.OP_WRITE; //we still have data to write
+        if (!flush(netOutBuffer)) {
+            return SelectionKey.OP_WRITE; //we still have data to write
+        }
 
         SSLEngineResult handshake = null;
 
         while (!handshakeComplete) {
-            switch ( handshakeStatus ) {
-                case NOT_HANDSHAKING: {
+            switch (handshakeStatus) {
+                case NOT_HANDSHAKING:
                     //should never happen
                     throw new IOException(sm.getString("channel.nio.ssl.notHandshaking"));
-                }
-                case FINISHED: {
+                case FINISHED:
                     if (endpoint.hasNegotiableProtocols()) {
                         if (sslEngine instanceof SSLUtil.ProtocolInfo) {
                             socketWrapper.setNegotiatedProtocol(
@@ -203,9 +204,8 @@ public class SecureNioChannel extends NioChannel {
                     //we are complete if we have delivered the last package
                     handshakeComplete = !netOutBuffer.hasRemaining();
                     //return 0 if we are complete, otherwise we still have data to write
-                    return handshakeComplete?0:SelectionKey.OP_WRITE;
-                }
-                case NEED_WRAP: {
+                    return handshakeComplete ? 0 : SelectionKey.OP_WRITE;
+                case NEED_WRAP:
                     //perform the wrap function
                     try {
                         handshake = handshakeWrap(write);
@@ -216,8 +216,9 @@ public class SecureNioChannel extends NioChannel {
                         handshake = handshakeWrap(write);
                     }
                     if (handshake.getStatus() == Status.OK) {
-                        if (handshakeStatus == HandshakeStatus.NEED_TASK)
+                        if (handshakeStatus == HandshakeStatus.NEED_TASK) {
                             handshakeStatus = tasks();
+                        }
                     } else if (handshake.getStatus() == Status.CLOSED) {
                         flush(netOutBuffer);
                         return -1;
@@ -225,33 +226,32 @@ public class SecureNioChannel extends NioChannel {
                         //wrap should always work with our buffers
                         throw new IOException(sm.getString("channel.nio.ssl.unexpectedStatusDuringWrap", handshake.getStatus()));
                     }
-                    if ( handshakeStatus != HandshakeStatus.NEED_UNWRAP || (!flush(netOutBuffer)) ) {
+                    if (handshakeStatus != HandshakeStatus.NEED_UNWRAP || (!flush(netOutBuffer))) {
                         //should actually return OP_READ if we have NEED_UNWRAP
                         return SelectionKey.OP_WRITE;
                     }
                     //fall down to NEED_UNWRAP on the same call, will result in a
                     //BUFFER_UNDERFLOW if it needs data
-                }
                 //$FALL-THROUGH$
-                case NEED_UNWRAP: {
+                case NEED_UNWRAP:
                     //perform the unwrap function
                     handshake = handshakeUnwrap(read);
-                    if ( handshake.getStatus() == Status.OK ) {
-                        if (handshakeStatus == HandshakeStatus.NEED_TASK)
+                    if (handshake.getStatus() == Status.OK) {
+                        if (handshakeStatus == HandshakeStatus.NEED_TASK) {
                             handshakeStatus = tasks();
+                        }
                     } else if ( handshake.getStatus() == Status.BUFFER_UNDERFLOW ){
                         //read more data, reregister for OP_READ
                         return SelectionKey.OP_READ;
                     } else {
                         throw new IOException(sm.getString("channel.nio.ssl.unexpectedStatusDuringWrap", handshake.getStatus()));
-                    }//switch
+                    }
                     break;
-                }
-                case NEED_TASK: {
+                case NEED_TASK:
                     handshakeStatus = tasks();
                     break;
-                }
-                default: throw new IllegalStateException(sm.getString("channel.nio.ssl.invalidStatus", handshakeStatus));
+                default:
+                    throw new IllegalStateException(sm.getString("channel.nio.ssl.invalidStatus", handshakeStatus));
             }
         }
         // Handshake is complete if this point is reached
@@ -363,10 +363,18 @@ public class SecureNioChannel extends NioChannel {
     @SuppressWarnings("null") // key cannot be null
     public void rehandshake(long timeout) throws IOException {
         //validate the network buffers are empty
-        if (netInBuffer.position() > 0 && netInBuffer.position()<netInBuffer.limit()) throw new IOException(sm.getString("channel.nio.ssl.netInputNotEmpty"));
-        if (netOutBuffer.position() > 0 && netOutBuffer.position()<netOutBuffer.limit()) throw new IOException(sm.getString("channel.nio.ssl.netOutputNotEmpty"));
-        if (!getBufHandler().isReadBufferEmpty()) throw new IOException(sm.getString("channel.nio.ssl.appInputNotEmpty"));
-        if (!getBufHandler().isWriteBufferEmpty()) throw new IOException(sm.getString("channel.nio.ssl.appOutputNotEmpty"));
+        if (netInBuffer.position() > 0 && netInBuffer.position() < netInBuffer.limit()) {
+            throw new IOException(sm.getString("channel.nio.ssl.netInputNotEmpty"));
+        }
+        if (netOutBuffer.position() > 0 && netOutBuffer.position() < netOutBuffer.limit()) {
+            throw new IOException(sm.getString("channel.nio.ssl.netOutputNotEmpty"));
+        }
+        if (!getBufHandler().isReadBufferEmpty()) {
+            throw new IOException(sm.getString("channel.nio.ssl.appInputNotEmpty"));
+        }
+        if (!getBufHandler().isWriteBufferEmpty()) {
+            throw new IOException(sm.getString("channel.nio.ssl.appOutputNotEmpty"));
+        }
         handshakeComplete = false;
         boolean isReadable = false;
         boolean isWriteable = false;
@@ -379,11 +387,14 @@ public class SecureNioChannel extends NioChannel {
             while (handshaking) {
                 int hsStatus = this.handshake(isReadable, isWriteable);
                 switch (hsStatus) {
-                    case -1 : throw new EOFException(sm.getString("channel.nio.ssl.eofDuringHandshake"));
-                    case  0 : handshaking = false; break;
-                    default : {
+                    case -1 :
+                        throw new EOFException(sm.getString("channel.nio.ssl.eofDuringHandshake"));
+                    case  0 :
+                        handshaking = false;
+                        break;
+                    default :
                         long now = System.currentTimeMillis();
-                        if (selector==null) {
+                        if (selector == null) {
                             selector = Selector.open();
                             key = getIOChannel().register(selector, hsStatus);
                         } else {
@@ -395,7 +406,6 @@ public class SecureNioChannel extends NioChannel {
                         }
                         isReadable = key.isReadable();
                         isWriteable = key.isWritable();
-                    }
                 }
             }
         } catch (IOException x) {
@@ -406,8 +416,18 @@ public class SecureNioChannel extends NioChannel {
             IOException x = new IOException(cx);
             throw x;
         } finally {
-            if (key!=null) try {key.cancel();} catch (Exception ignore) {}
-            if (selector!=null) try {selector.close();} catch (Exception ignore) {}
+            if (key != null) {
+                try {
+                    key.cancel();
+                } catch (Exception ignore) {
+                }
+            }
+            if (selector != null) {
+                try {
+                    selector.close();
+                } catch (Exception ignore) {
+                }
+            }
         }
     }
 
@@ -419,7 +439,7 @@ public class SecureNioChannel extends NioChannel {
      */
     protected SSLEngineResult.HandshakeStatus tasks() {
         Runnable r = null;
-        while ( (r = sslEngine.getDelegatedTask()) != null) {
+        while ((r = sslEngine.getDelegatedTask()) != null) {
             r.run();
         }
         return sslEngine.getHandshakeStatus();
@@ -443,7 +463,9 @@ public class SecureNioChannel extends NioChannel {
         //set the status
         handshakeStatus = result.getHandshakeStatus();
         //optimization, if we do have a writable channel, write it now
-        if ( doWrite ) flush(netOutBuffer);
+        if (doWrite) {
+            flush(netOutBuffer);
+        }
         return result;
     }
 
@@ -459,10 +481,12 @@ public class SecureNioChannel extends NioChannel {
             //clear the buffer if we have emptied it out on data
             netInBuffer.clear();
         }
-        if ( doread )  {
+        if (doread)  {
             //if we have data to read, read it
             int read = sc.read(netInBuffer);
-            if (read == -1) throw new IOException(sm.getString("channel.nio.ssl.eofDuringHandshake"));
+            if (read == -1) {
+                throw new IOException(sm.getString("channel.nio.ssl.eofDuringHandshake"));
+            }
         }
         SSLEngineResult result;
         boolean cont = false;
@@ -477,15 +501,15 @@ public class SecureNioChannel extends NioChannel {
             netInBuffer.compact();
             //read in the status
             handshakeStatus = result.getHandshakeStatus();
-            if ( result.getStatus() == SSLEngineResult.Status.OK &&
-                 result.getHandshakeStatus() == HandshakeStatus.NEED_TASK ) {
+            if (result.getStatus() == SSLEngineResult.Status.OK &&
+                 result.getHandshakeStatus() == HandshakeStatus.NEED_TASK) {
                 //execute tasks if we need to
                 handshakeStatus = tasks();
             }
             //perform another unwrap?
             cont = result.getStatus() == SSLEngineResult.Status.OK &&
                    handshakeStatus == HandshakeStatus.NEED_UNWRAP;
-        }while ( cont );
+        } while (cont);
         return result;
     }
 
@@ -503,7 +527,9 @@ public class SecureNioChannel extends NioChannel {
      */
     @Override
     public void close() throws IOException {
-        if (closing) return;
+        if (closing) {
+            return;
+        }
         closing = true;
         sslEngine.closeOutbound();
 
@@ -565,14 +591,20 @@ public class SecureNioChannel extends NioChannel {
     @Override
     public int read(ByteBuffer dst) throws IOException {
         //are we in the middle of closing or closed?
-        if ( closing || closed) return -1;
+        if (closing || closed) {
+            return -1;
+        }
         //did we finish our handshake?
-        if (!handshakeComplete) throw new IllegalStateException(sm.getString("channel.nio.ssl.incompleteHandshake"));
+        if (!handshakeComplete) {
+            throw new IllegalStateException(sm.getString("channel.nio.ssl.incompleteHandshake"));
+        }
 
         //read from the network
         int netread = sc.read(netInBuffer);
         //did we reach EOF? if so send EOF up one layer.
-        if (netread == -1) return -1;
+        if (netread == -1) {
+            return -1;
+        }
 
         //the data read
         int read = 0;
@@ -632,14 +664,20 @@ public class SecureNioChannel extends NioChannel {
     public long read(ByteBuffer[] dsts, int offset, int length)
             throws IOException {
         //are we in the middle of closing or closed?
-        if ( closing || closed) return -1;
+        if (closing || closed) {
+            return -1;
+        }
         //did we finish our handshake?
-        if (!handshakeComplete) throw new IllegalStateException(sm.getString("channel.nio.ssl.incompleteHandshake"));
+        if (!handshakeComplete) {
+            throw new IllegalStateException(sm.getString("channel.nio.ssl.incompleteHandshake"));
+        }
 
         //read from the network
         int netread = sc.read(netInBuffer);
         //did we reach EOF? if so send EOF up one layer.
-        if (netread == -1) return -1;
+        if (netread == -1) {
+            return -1;
+        }
 
         //the data read
         int read = 0;
@@ -768,7 +806,9 @@ public class SecureNioChannel extends NioChannel {
             netOutBuffer.flip();
 
             if (result.getStatus() == Status.OK) {
-                if (result.getHandshakeStatus() == HandshakeStatus.NEED_TASK) tasks();
+                if (result.getHandshakeStatus() == HandshakeStatus.NEED_TASK) {
+                    tasks();
+                }
             } else {
                 throw new IOException(sm.getString("channel.nio.ssl.wrapFail", result.getStatus()));
             }
@@ -823,7 +863,7 @@ public class SecureNioChannel extends NioChannel {
     public boolean flushOutbound() throws IOException {
         int remaining = netOutBuffer.remaining();
         flush(netOutBuffer);
-        int remaining2= netOutBuffer.remaining();
+        int remaining2 = netOutBuffer.remaining();
         return remaining2 < remaining;
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org