Skip to content

Commit affc99f

Browse files
committed
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: #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.
1 parent 9da4ee0 commit affc99f

File tree

4 files changed

+17
-23
lines changed

4 files changed

+17
-23
lines changed

gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
group = org.threadly
22
version = 4.13
3-
threadlyVersion = 5.41
3+
threadlyVersion = 5.42

src/main/java/org/threadly/litesockets/buffers/AbstractMergedByteBuffers.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,15 @@ public void add(final byte[] ...bas) {
5858
@Override
5959
public void add(final ByteBuffer ...buffers) {
6060
for(ByteBuffer buffer: buffers) {
61-
if(buffer.hasRemaining()) {
62-
doAppend(buffer.duplicate());
63-
}
61+
doAppend(buffer);
6462
}
6563
}
6664

6765
@Override
6866
public void add(final MergedByteBuffers ...mbbs) {
6967
for(MergedByteBuffers mbb: mbbs) {
7068
while(mbb.hasRemaining()) {
71-
ByteBuffer bb = mbb.popBuffer();
72-
if(bb.hasRemaining()) {
73-
doAppend(bb);
74-
}
69+
doAppend(mbb.popBuffer());
7570
}
7671
}
7772
}

src/main/java/org/threadly/litesockets/buffers/ReuseableMergedByteBuffers.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,17 @@ public ReuseableMergedByteBuffers(boolean readOnly, ByteBuffer ...bbs) {
3939

4040
@Override
4141
protected void doAppend(final ByteBuffer bb) {
42-
availableBuffers.add(bb);
43-
currentSize+=bb.remaining();
42+
if (bb.hasRemaining()) {
43+
availableBuffers.add(bb.duplicate());
44+
currentSize+=bb.remaining();
45+
}
4446
}
4547

4648
@Override
4749
public ReuseableMergedByteBuffers duplicate() {
4850
final ReuseableMergedByteBuffers mbb = new ReuseableMergedByteBuffers(markReadOnly);
4951
for(final ByteBuffer bb: this.availableBuffers) {
50-
mbb.add(bb.duplicate());
52+
mbb.doAppend(bb);
5153
}
5254
return mbb;
5355
}
@@ -61,7 +63,7 @@ public ReuseableMergedByteBuffers duplicateAndClean() {
6163

6264
@Override
6365
protected void addToFront(ByteBuffer bb) {
64-
this.availableBuffers.addFirst(bb.duplicate());
66+
this.availableBuffers.addFirst(bb);
6567
this.currentSize+=bb.remaining();
6668
}
6769

src/main/java/org/threadly/litesockets/buffers/SimpleMergedByteBuffers.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,18 @@
1414
public class SimpleMergedByteBuffers extends AbstractMergedByteBuffers {
1515
private static final ByteBuffer[] EMPTY_BUFFER_ARRAY = new ByteBuffer[] {};
1616

17-
1817
private final ByteBuffer[] bba;
1918
private int currentBuffer = 0;
2019
protected long consumedSize = 0;
2120

2221
public SimpleMergedByteBuffers(boolean readOnly, ByteBuffer ...bbs) {
2322
super(readOnly);
24-
for(ByteBuffer bb: bbs) {
25-
if(bb == null) {
23+
24+
for (int i = 0; i < bbs.length; i++) {
25+
if(bbs[i] == null) {
2626
throw new IllegalArgumentException("Can not add null buffers!");
2727
}
28+
bbs[i] = bbs[i].duplicate();
2829
}
2930
if(bbs.length > 0) {
3031
bba = bbs;
@@ -50,10 +51,6 @@ public SimpleMergedByteBuffers(boolean readOnly, SimpleMergedByteBuffers smbb, B
5051
}
5152
}
5253

53-
private void doGet(final byte[] destBytes) {
54-
doGet(destBytes, 0, destBytes.length);
55-
}
56-
5754
private void doGet(final byte[] destBytes, int start, int len) {
5855
int remainingToCopy = len;
5956

@@ -88,7 +85,7 @@ protected void addToFront(ByteBuffer bb) {
8885
public SimpleMergedByteBuffers duplicate() {
8986
ByteBuffer[] bba2 = new ByteBuffer[bba.length-currentBuffer];
9087
for(int i=currentBuffer; i<bba.length; i++) {
91-
bba2[i-currentBuffer] = bba[i].duplicate();
88+
bba2[i-currentBuffer] = bba[i];
9289
}
9390
return new SimpleMergedByteBuffers(markReadOnly, bba2);
9491
}
@@ -97,7 +94,7 @@ public SimpleMergedByteBuffers duplicate() {
9794
public SimpleMergedByteBuffers duplicateAndClean() {
9895
SimpleMergedByteBuffers smbb = duplicate();
9996
currentBuffer = bba.length;
100-
for(int i=0; i<bba.length; i++) {
97+
for(int i=currentBuffer; i<bba.length; i++) {
10198
bba[i] = null;
10299
}
103100
return smbb;
@@ -170,15 +167,15 @@ public ByteBuffer pullBuffer(int size) {
170167
final ByteBuffer first = getNextBuffer();
171168
if(first.remaining() == size) {
172169
currentBuffer++;
173-
return first.duplicate();
170+
return first;
174171
} else if(first.remaining() > size) {
175172
final ByteBuffer bb = first.duplicate();
176173
bb.limit(bb.position()+size);
177174
first.position(first.position()+size);
178175
return bb;
179176
} else {
180177
final byte[] result = new byte[size];
181-
doGet(result);
178+
doGet(result, 0, size);
182179
return ByteBuffer.wrap(result);
183180
}
184181
}

0 commit comments

Comments
 (0)