From affc99ff563da1ea59798983edd4c492207a286a Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Thu, 13 Feb 2020 15:53:34 -0700 Subject: [PATCH 1/2] SSL hang / timeout fix This fixes ssl, which can be seen using litesockets-http-client when using https. The behavior would be the start of the handshake which would eventually hang. This regression was introduced in this PR: https://github.com/threadly/litesockets/pull/71 The specific flaw was introduced when the .duplicate() was removed here: https://github.com/threadly/litesockets/pull/71/files#diff-851d3597731b2cecc83331ede7aec7ebL143 It seems for SSL to work correctly we need to make sure that the MergedByteBuffer does not work against the original ByteBuffer that was originally passed in. This fixes that behavior by making sure that when ByteBuffers are added (or provided at construction), we do this .duplicate() then. This way when buffers are leaving the MergedByteBuffer we know it already has an independent experience from the one provided initially. The goal being that by having this duplicate action happen at a consistent point will make this safer. Overall I don't think we really add any .duplicate() actions, if anything at times we might have less. With it happening at the start we can safely avoid duplicate calls at other points we had them previously. --- gradle.properties | 2 +- .../buffers/AbstractMergedByteBuffers.java | 9 ++------- .../buffers/ReuseableMergedByteBuffers.java | 10 ++++++---- .../buffers/SimpleMergedByteBuffers.java | 19 ++++++++----------- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/gradle.properties b/gradle.properties index a52943c..53eb693 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ group = org.threadly version = 4.13 -threadlyVersion = 5.41 +threadlyVersion = 5.42 diff --git a/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java index bbacae3..e10c520 100644 --- a/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java @@ -58,9 +58,7 @@ public void add(final byte[] ...bas) { @Override public void add(final ByteBuffer ...buffers) { for(ByteBuffer buffer: buffers) { - if(buffer.hasRemaining()) { - doAppend(buffer.duplicate()); - } + doAppend(buffer); } } @@ -68,10 +66,7 @@ public void add(final ByteBuffer ...buffers) { public void add(final MergedByteBuffers ...mbbs) { for(MergedByteBuffers mbb: mbbs) { while(mbb.hasRemaining()) { - ByteBuffer bb = mbb.popBuffer(); - if(bb.hasRemaining()) { - doAppend(bb); - } + doAppend(mbb.popBuffer()); } } } diff --git a/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java index 37c13f9..08346ea 100644 --- a/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java @@ -39,15 +39,17 @@ public ReuseableMergedByteBuffers(boolean readOnly, ByteBuffer ...bbs) { @Override protected void doAppend(final ByteBuffer bb) { - availableBuffers.add(bb); - currentSize+=bb.remaining(); + if (bb.hasRemaining()) { + availableBuffers.add(bb.duplicate()); + currentSize+=bb.remaining(); + } } @Override public ReuseableMergedByteBuffers duplicate() { final ReuseableMergedByteBuffers mbb = new ReuseableMergedByteBuffers(markReadOnly); for(final ByteBuffer bb: this.availableBuffers) { - mbb.add(bb.duplicate()); + mbb.doAppend(bb); } return mbb; } @@ -61,7 +63,7 @@ public ReuseableMergedByteBuffers duplicateAndClean() { @Override protected void addToFront(ByteBuffer bb) { - this.availableBuffers.addFirst(bb.duplicate()); + this.availableBuffers.addFirst(bb); this.currentSize+=bb.remaining(); } diff --git a/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java index 4a5749a..37b7eca 100644 --- a/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java @@ -14,17 +14,18 @@ public class SimpleMergedByteBuffers extends AbstractMergedByteBuffers { private static final ByteBuffer[] EMPTY_BUFFER_ARRAY = new ByteBuffer[] {}; - private final ByteBuffer[] bba; private int currentBuffer = 0; protected long consumedSize = 0; public SimpleMergedByteBuffers(boolean readOnly, ByteBuffer ...bbs) { super(readOnly); - for(ByteBuffer bb: bbs) { - if(bb == null) { + + for (int i = 0; i < bbs.length; i++) { + if(bbs[i] == null) { throw new IllegalArgumentException("Can not add null buffers!"); } + bbs[i] = bbs[i].duplicate(); } if(bbs.length > 0) { bba = bbs; @@ -50,10 +51,6 @@ public SimpleMergedByteBuffers(boolean readOnly, SimpleMergedByteBuffers smbb, B } } - private void doGet(final byte[] destBytes) { - doGet(destBytes, 0, destBytes.length); - } - private void doGet(final byte[] destBytes, int start, int len) { int remainingToCopy = len; @@ -88,7 +85,7 @@ protected void addToFront(ByteBuffer bb) { public SimpleMergedByteBuffers duplicate() { ByteBuffer[] bba2 = new ByteBuffer[bba.length-currentBuffer]; for(int i=currentBuffer; i size) { final ByteBuffer bb = first.duplicate(); bb.limit(bb.position()+size); @@ -178,7 +175,7 @@ public ByteBuffer pullBuffer(int size) { return bb; } else { final byte[] result = new byte[size]; - doGet(result); + doGet(result, 0, size); return ByteBuffer.wrap(result); } } From 400e5042997fccfdbb868839160891a8dab10273 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Fri, 14 Feb 2020 08:28:56 -0700 Subject: [PATCH 2/2] Remove unused AbstractMergedByteBuffers.addToFront --- .../litesockets/buffers/AbstractMergedByteBuffers.java | 1 - .../litesockets/buffers/ReuseableMergedByteBuffers.java | 6 ------ .../litesockets/buffers/SimpleMergedByteBuffers.java | 5 ----- 3 files changed, 12 deletions(-) diff --git a/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java index e10c520..75978ba 100644 --- a/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java @@ -31,7 +31,6 @@ public AbstractMergedByteBuffers(boolean readOnly) { } protected abstract void doAppend(final ByteBuffer bb); - protected abstract void addToFront(final ByteBuffer bb); protected abstract byte get(int pos); public abstract AbstractMergedByteBuffers duplicate(); public abstract AbstractMergedByteBuffers duplicateAndClean(); diff --git a/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java index 08346ea..9474c2a 100644 --- a/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java @@ -61,12 +61,6 @@ public ReuseableMergedByteBuffers duplicateAndClean() { return mbb; } - @Override - protected void addToFront(ByteBuffer bb) { - this.availableBuffers.addFirst(bb); - this.currentSize+=bb.remaining(); - } - @Override public int remaining() { return currentSize; diff --git a/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java b/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java index 37b7eca..d07d074 100644 --- a/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java +++ b/src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java @@ -76,11 +76,6 @@ protected void doAppend(ByteBuffer bb) { throw new UnsupportedOperationException("Can not add to this buffer!"); } - @Override - protected void addToFront(ByteBuffer bb) { - throw new UnsupportedOperationException("Can not add to this buffer!"); - } - @Override public SimpleMergedByteBuffers duplicate() { ByteBuffer[] bba2 = new ByteBuffer[bba.length-currentBuffer];