From 186e9dac2b4d7d98f49b73e7cfa406b58cc64b46 Mon Sep 17 00:00:00 2001 From: Vinod Mukkamala Date: Sun, 5 Nov 2017 21:11:50 -0500 Subject: [PATCH] Simple Framework Fixes 1. Bumped version to 6.0.2 2. Bumped Java Source and Targer version from 1.5 to 1.6 3. FileBuffer: convert has potential bug; simplified read() method 4. Handshake: improved receive() method 5: SecureTransport: Fixed Bugs 5.1: receive() method where unwrap is only called when swap buffer has data. Should call unwarp till SSLEngine has consumed and produced all bytes it could. Noticed Bug when uploading huge files ( > 1 MB ) 5.2: send() method could potentially have same issue when wrapping. 5.3: write() method improvements due changes in send() method 6: TransportReader: Fixed a potential bug where count is directly adjusted without checking read result --- simple/pom.xml | 2 +- simple/simple-common/build.properties | 2 +- simple/simple-common/pom.xml | 6 +- simple/simple-common/script/pom.xml | 4 +- .../common/buffer/FileBuffer.java | 15 +- simple/simple-http/build.properties | 2 +- simple/simple-http/pom.xml | 10 +- simple/simple-http/script/pom.xml | 4 +- .../http/core/RequestCollector.java | 1 - simple/simple-transport/build.properties | 2 +- simple/simple-transport/pom.xml | 8 +- simple/simple-transport/script/pom.xml | 4 +- .../simpleframework/transport/Handshake.java | 10 +- .../transport/NegotiationState.java | 4 +- .../transport/SecureTransport.java | 164 +++++++----------- .../transport/SocketBuffer.java | 15 +- .../transport/TransportCursor.java | 2 +- .../transport/TransportReader.java | 11 +- 18 files changed, 116 insertions(+), 150 deletions(-) diff --git a/simple/pom.xml b/simple/pom.xml index 77d2020..b77fe23 100644 --- a/simple/pom.xml +++ b/simple/pom.xml @@ -3,7 +3,7 @@ org.simpleframework simple pom - 6.0.1 + 6.0.2 Simple simple-common diff --git a/simple/simple-common/build.properties b/simple/simple-common/build.properties index d7b1be1..6fb5f1b 100644 --- a/simple/simple-common/build.properties +++ b/simple/simple-common/build.properties @@ -1,3 +1,3 @@ -build.version=6.0.1 +build.version=6.0.2 build.name=simple-common build.description=Simple Common diff --git a/simple/simple-common/pom.xml b/simple/simple-common/pom.xml index e1a4b3a..64f2fe9 100644 --- a/simple/simple-common/pom.xml +++ b/simple/simple-common/pom.xml @@ -8,7 +8,7 @@ org.simpleframework simple-common jar - 6.0.1 + 6.0.2 Simple Common http://www.simpleframework.org Simple is a high performance asynchronous HTTP framework for Java @@ -65,8 +65,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-common/script/pom.xml b/simple/simple-common/script/pom.xml index aee44cf..06c5d6e 100644 --- a/simple/simple-common/script/pom.xml +++ b/simple/simple-common/script/pom.xml @@ -65,8 +65,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-common/src/main/java/org/simpleframework/common/buffer/FileBuffer.java b/simple/simple-common/src/main/java/org/simpleframework/common/buffer/FileBuffer.java index 8fcea77..7efd6d5 100644 --- a/simple/simple-common/src/main/java/org/simpleframework/common/buffer/FileBuffer.java +++ b/simple/simple-common/src/main/java/org/simpleframework/common/buffer/FileBuffer.java @@ -188,15 +188,14 @@ public String encode(String charset) throws IOException { */ private String convert(InputStream source, String charset, int count) throws IOException { byte[] buffer = new byte[count]; - int left = count; - while(left > 0) { - int size = source.read(buffer, 0, left); + int read; + for(int offset = 0; offset < count; offset+=read) { + read = source.read(buffer, offset, count-offset); - if(size == -1) { + if(read <= -1) { throw new BufferException("Could not read buffer"); } - left -= count; } return new String(buffer, charset); } @@ -530,10 +529,10 @@ public Range(InputStream source, long length) { */ @Override public int read() throws IOException { - if(length-- > 0) { + if(length >= 1) { + length--; return in.read(); - } - if(length <= 0) { + } else { close(); } return -1; diff --git a/simple/simple-http/build.properties b/simple/simple-http/build.properties index 7e2b5ea..681450d 100644 --- a/simple/simple-http/build.properties +++ b/simple/simple-http/build.properties @@ -1,3 +1,3 @@ -build.version=6.0.1 +build.version=6.0.2 build.name=simple-http build.description=Simple HTTP diff --git a/simple/simple-http/pom.xml b/simple/simple-http/pom.xml index 2a079fc..e8b11e6 100644 --- a/simple/simple-http/pom.xml +++ b/simple/simple-http/pom.xml @@ -8,7 +8,7 @@ org.simpleframework simple-http jar - 6.0.1 + 6.0.2 Simple HTTP http://www.simpleframework.org Simple is a high performance asynchronous HTTP framework for Java @@ -40,12 +40,12 @@ org.simpleframework simple-common - 6.0.1 + 6.0.2 org.simpleframework simple-transport - 6.0.1 + 6.0.2 junit @@ -75,8 +75,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-http/script/pom.xml b/simple/simple-http/script/pom.xml index dfb7df4..c917b77 100644 --- a/simple/simple-http/script/pom.xml +++ b/simple/simple-http/script/pom.xml @@ -75,8 +75,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-http/src/main/java/org/simpleframework/http/core/RequestCollector.java b/simple/simple-http/src/main/java/org/simpleframework/http/core/RequestCollector.java index 027318d..ada2ecf 100644 --- a/simple/simple-http/src/main/java/org/simpleframework/http/core/RequestCollector.java +++ b/simple/simple-http/src/main/java/org/simpleframework/http/core/RequestCollector.java @@ -78,7 +78,6 @@ class RequestCollector implements Collector { * an internal buffer to store the consumed body. * * @param allocator this is the allocator used to buffer data - * @param tracker this is the tracker used to create sessions * @param channel this is the channel used to read the data */ public RequestCollector(Allocator allocator, Channel channel) { diff --git a/simple/simple-transport/build.properties b/simple/simple-transport/build.properties index ccdc2d2..df6307e 100644 --- a/simple/simple-transport/build.properties +++ b/simple/simple-transport/build.properties @@ -1,3 +1,3 @@ -build.version=6.0.1 +build.version=6.0.2 build.name=simple-transport build.description=Simple Transport diff --git a/simple/simple-transport/pom.xml b/simple/simple-transport/pom.xml index 061e5b5..0048212 100644 --- a/simple/simple-transport/pom.xml +++ b/simple/simple-transport/pom.xml @@ -8,7 +8,7 @@ org.simpleframework simple-transport jar - 6.0.1 + 6.0.2 Simple Transport http://www.simpleframework.org Simple is a high performance asynchronous HTTP framework for Java @@ -40,7 +40,7 @@ org.simpleframework simple-common - 6.0.1 + 6.0.2 junit @@ -70,8 +70,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-transport/script/pom.xml b/simple/simple-transport/script/pom.xml index f6a8869..a6edcca 100644 --- a/simple/simple-transport/script/pom.xml +++ b/simple/simple-transport/script/pom.xml @@ -70,8 +70,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/Handshake.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/Handshake.java index 2a4b9a2..b122573 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/Handshake.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/Handshake.java @@ -431,12 +431,9 @@ private void execute() throws IOException { * @return this returns true when the message has been read */ public boolean receive() throws IOException { - int count = input.capacity(); - - if(count > 0) { - input.compact(); - } + input.compact(); // maximize read buffer int size = channel.read(input); + input.flip(); if(trace != null) { trace.trace(READ, size); @@ -444,9 +441,6 @@ public boolean receive() throws IOException { if(size < 0) { throw new TransportException("Client closed connection"); } - if(count > 0) { - input.flip(); - } return size > 0; } diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/NegotiationState.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/NegotiationState.java index 160b5f9..134d830 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/NegotiationState.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/NegotiationState.java @@ -217,7 +217,7 @@ public Future challenge() { * completion of the SSL renegotiation results in the client * providing their certificate, and execution of the task. * - * @param completion task to be run on successful challenge + * @param task to be run on successful challenge */ public Future challenge(Runnable task) { try { @@ -238,7 +238,7 @@ public Future challenge(Runnable task) { * completion of the SSL renegotiation results in the client * providing their certificate, and execution of the task. * - * @param completion task to be run on successful challenge + * @param task to be run on successful challenge */ private void resume(Runnable task) { try { diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/SecureTransport.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/SecureTransport.java index 2083873..ff8de34 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/SecureTransport.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/SecureTransport.java @@ -199,16 +199,20 @@ public SocketChannel getChannel() { * @return this returns the number of bytes that have been read */ public int read(ByteBuffer buffer) throws IOException { - if(closed) { - throw new TransportException("Transport is closed"); - } - if(finished) { - return -1; + + int count = fill(buffer); + + if (count <= 0) { + count = process(buffer); } - int count = fill(buffer); - - if(count <= 0) { - return process(buffer); + + if (count <= 0) { + if(closed) { + throw new TransportException("Transport is closed"); + } + if(finished) { + return -1; + } } return count; } @@ -225,24 +229,8 @@ public int read(ByteBuffer buffer) throws IOException { * @return this returns the number of bytes that have been read */ private int process(ByteBuffer buffer) throws IOException { - int size = swap.position(); - - if(size >= 0) { - swap.compact(); - } - int space = swap.remaining(); - - if(space > 0) { - size = transport.read(swap); - - if(size < 0) { - finished = true; - } - } - if(size > 0 || space > 0) { - swap.flip(); - receive(); - } + receive(); + return fill(buffer); } @@ -257,35 +245,11 @@ private int process(ByteBuffer buffer) throws IOException { * @return this returns the number of bytes that have been read */ private int fill(ByteBuffer buffer) throws IOException { - int space = buffer.remaining(); - int count = input.position(); - - if(count > 0) { - if(count > space) { - count = space; - } - } - return fill(buffer, count); - - } - - /** - * This is used to fill the provided buffer with data that has - * been read from the secure socket channel. This enables reading - * of the decrypted data in chunks that are smaller than the - * size of the input buffer used to contain the plain text data. - * - * @param buffer this is the buffer to append the bytes to - * @param count this is the number of bytes that are to be read - * - * @return this returns the number of bytes that have been read - */ - private int fill(ByteBuffer buffer, int count) throws IOException { input.flip(); + int count = Math.min(input.remaining(), buffer.remaining()); - if(count > 0) { - count = append(buffer, count); - } + count = append(buffer, count); + input.compact(); return count; } @@ -302,16 +266,14 @@ private int fill(ByteBuffer buffer, int count) throws IOException { * @return returns the number of bytes that have been moved */ private int append(ByteBuffer buffer, int count) throws IOException { - ByteBuffer segment = input.slice(); - if(closed) { - throw new TransportException("Transport is closed"); - } - int mark = input.position(); - int size = mark + count; - if(count > 0) { + ByteBuffer segment = input.slice(); + + int mark = input.position(); + int size = mark + count; input.position(size); + segment.limit(count); buffer.put(segment); } @@ -326,26 +288,37 @@ private int append(ByteBuffer buffer, int count) throws IOException { * will return the number of bytes read. Finally if the socket * is closed this will return a -1 value. */ - private void receive() throws IOException { - int count = swap.remaining(); + private int receive() throws IOException { + int bytesProduced = 0; + boolean exitWhile = false; + while(!exitWhile) { + // Read from transport + swap.compact(); + int read = transport.read(swap); + swap.flip(); - while(count > 0) { SSLEngineResult result = engine.unwrap(swap, input); Status status = result.getStatus(); + + bytesProduced += result.bytesProduced(); switch(status) { + case OK: + break; + case CLOSED: + finished = true; + // no break case BUFFER_OVERFLOW: case BUFFER_UNDERFLOW: - return; - case CLOSED: - throw new TransportException("Transport error " + result); + exitWhile = true; + break; } - count = swap.remaining(); - if(count <= 0) { - break; + if (read < 0) { + finished = true; } } + return (bytesProduced == 0 && finished)? -1 : bytesProduced; } /** @@ -359,22 +332,9 @@ private void receive() throws IOException { public void write(ByteBuffer buffer) throws IOException { if(closed) { throw new TransportException("Transport is closed"); - } - int capacity = output.capacity(); - int ready = buffer.remaining(); - int length = ready; - - while(ready > 0) { - int size = Math.min(ready, capacity / 2); - int mark = buffer.position(); - - if(length * 2 > capacity) { - buffer.limit(mark + size); - } - send(buffer); - output.clear(); - ready -= size; } + + send(buffer); } /** @@ -385,19 +345,27 @@ public void write(ByteBuffer buffer) throws IOException { * * @param buffer this is the array of bytes to send to the client */ - private void send(ByteBuffer buffer) throws IOException { - SSLEngineResult result = engine.wrap(buffer, output); - Status status = result.getStatus(); - - switch(status){ - case BUFFER_OVERFLOW: - case BUFFER_UNDERFLOW: - case CLOSED: - throw new TransportException("Transport error " + status); - default: - output.flip(); - } - transport.write(output); + private void send(ByteBuffer buffer) throws IOException { + boolean exitWhile = false; + while (!exitWhile) { + SSLEngineResult result = engine.wrap(buffer, output); + Status status = result.getStatus(); + + switch (status) { + case BUFFER_OVERFLOW: + case BUFFER_UNDERFLOW: + exitWhile = true; + break; + case CLOSED: + throw new TransportException("Transport error " + status); + case OK: + exitWhile = ((result.bytesProduced()==0) && ! buffer.hasRemaining()); + break; + } + output.flip(); + transport.write(output); + output.compact(); + } } /** diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/SocketBuffer.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/SocketBuffer.java index d3cdc22..86f13d1 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/SocketBuffer.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/SocketBuffer.java @@ -125,7 +125,7 @@ public synchronized boolean ready() throws IOException { * * @return this returns true if no reference was held */ - public synchronized boolean write(ByteBuffer duplicate) throws IOException { + public synchronized boolean write(ByteBuffer data) throws IOException { if(closed) { throw new TransportException("Buffer has been closed"); } @@ -135,22 +135,22 @@ public synchronized boolean write(ByteBuffer duplicate) throws IOException { int count = appender.length(); if(count > 0) { - return merge(duplicate); + return merge(data); } - int remaining = duplicate.remaining(); + int remaining = data.remaining(); if(remaining < chunk) { - appender.append(duplicate);// just save it.. + appender.append(data);// just save it.. return true; } - if(!flush(duplicate)) { // attempt a write + if(!flush(data)) { // attempt a write int space = appender.space(); if(remaining < space) { - appender.append(duplicate); + appender.append(data); return true; } - reference = duplicate; + reference = data; return false; } return true; @@ -230,7 +230,6 @@ public synchronized boolean flush() throws IOException { * can be acquired from the length method. Once all * of the bytes are written the packet must be closed. * - * @param channel this is the channel to write the packet to * @param segment this is the segment that is to be written * * @return this returns the number of bytes that were written diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportCursor.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportCursor.java index d25cb90..11c46ab 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportCursor.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportCursor.java @@ -73,7 +73,7 @@ public class TransportCursor implements ByteCursor { * @param transport this is the underlying transport to use */ public TransportCursor(Transport transport) { - this(transport, 2048); + this(transport, 20*2048); } /** diff --git a/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportReader.java b/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportReader.java index 713c162..6957e82 100644 --- a/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportReader.java +++ b/simple/simple-transport/src/main/java/org/simpleframework/transport/TransportReader.java @@ -176,12 +176,16 @@ private int peek() throws IOException { if(count > 0) { buffer.compact(); // compact the buffer } - count += transport.read(buffer); // how many were read + int read = transport.read(buffer); // how many were read + + if(read > 0) { + count += read; + } if(count > 0) { buffer.flip(); // if there is something then flip } - if(count < 0) { // close when stream is fully read + if(read < 0 && count <= 0) { // close when stream is fully read close(); } return count; @@ -198,6 +202,9 @@ private int peek() throws IOException { * @return this is the number of bytes that have been reset */ public int reset(int size) throws IOException { + if (size <=0) { + throw new IllegalArgumentException("invalid size " + size + " while resetting"); + } int mark = buffer.position(); if(size > mark) {