You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by sb...@apache.org on 2008/06/09 22:47:43 UTC
svn commit: r665886 - in /httpcomponents/httpcore/trunk: ./
module-nio/src/main/java/org/apache/http/impl/nio/codecs/
module-nio/src/main/java/org/apache/http/nio/
module-nio/src/test/java/org/apache/http/impl/nio/codecs/
Author: sberlin
Date: Mon Jun 9 13:47:42 2008
New Revision: 665886
URL: http://svn.apache.org/viewvc?rev=665886&view=rev
Log:
fix two bugs w/ FileContentDecoder.transfer.
a) if data was buffered in SessionInputBuffer, arbitrary parts of the file got overwritten
b) attempts to write beyond the length of the file silently failed, leading to easy spinning.
Modified:
httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/IdentityDecoder.java
httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/FileContentDecoder.java
httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestIdentityDecoder.java
httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
Modified: httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Mon Jun 9 13:47:42 2008
@@ -1,5 +1,15 @@
Changes since 4.0 Beta 1
-------------------
+* Changed behavior of IdentityDecoder & LengthDelimitedDecoder to throw
+ an IOException if data is attempted to be written beyond the length
+ of a FileChannel. Previously would write nothing.
+ Sam Berlin <sberlin at apache.org>
+
+* Fixed bug in LengthDelimitedDecoder & IdentityDecoder that caused transfers
+ to a FileChannel to overwrite arbitrary parts of the file, if data was
+ buffered in SessionInputBuffer.
+ Sam Berlin <sberlin at apache.org>
+
* Fixed concurrency bug in the ThrottlingHttpServerHandler protocol handler.
Oleg Kalnichevski <olegk at apache.org>
Modified: httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/IdentityDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/IdentityDecoder.java?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/IdentityDecoder.java (original)
+++ httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/IdentityDecoder.java Mon Jun 9 13:47:42 2008
@@ -97,9 +97,15 @@
long bytesRead;
if (this.buffer.hasData()) {
+ dst.position(position);
bytesRead = this.buffer.read(dst);
} else {
if (this.channel.isOpen()) {
+ if(dst.size() < position)
+ throw new IOException("FileChannel.size() [" + dst.size() +
+ "] < position [" + position +
+ "]. Please grow the file before writing.");
+
bytesRead = dst.transferFrom(this.channel, position, count);
} else {
bytesRead = -1;
Modified: httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java (original)
+++ httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java Mon Jun 9 13:47:42 2008
@@ -124,12 +124,18 @@
long bytesRead;
if (this.buffer.hasData()) {
int maxLen = Math.min(lenRemaining, this.buffer.length());
+ dst.position(position);
bytesRead = this.buffer.read(dst, maxLen);
} else {
if (count > lenRemaining) {
count = lenRemaining;
}
if (this.channel.isOpen()) {
+ if(dst.size() < position)
+ throw new IOException("FileChannel.size() [" + dst.size() +
+ "] < position [" + position +
+ "]. Please grow the file before writing.");
+
bytesRead = dst.transferFrom(this.channel, position, count);
} else {
bytesRead = -1;
Modified: httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/FileContentDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/FileContentDecoder.java?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/FileContentDecoder.java (original)
+++ httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/FileContentDecoder.java Mon Jun 9 13:47:42 2008
@@ -41,12 +41,17 @@
/**
* Transfers a portion of entity content from the underlying network channel
- * into the given file channel.
+ * into the given file channel.<br>
+ *
+ * <b>Warning</b>: Many implementations cannot write beyond the length of the file.
+ * If the position exceeds the channel's size, some implementations
+ * may throw an IOException.
*
* @param dst the target FileChannel to transfer data into.
* @param position
* The position within the file at which the transfer is to begin;
- * must be non-negative
+ * must be non-negative.
+ * <b>Must be less than or equal to the size of the file</b>
* @param count
* The maximum number of bytes to be transferred; must be
* non-negative
Modified: httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestIdentityDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestIdentityDecoder.java?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestIdentityDecoder.java (original)
+++ httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestIdentityDecoder.java Mon Jun 9 13:47:42 2008
@@ -32,6 +32,7 @@
import java.io.File;
import java.io.FileInputStream;
+import java.io.IOException;
import java.io.InputStreamReader;
import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
@@ -115,22 +116,26 @@
assertEquals(6, bytesRead);
assertEquals("stuff;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(6, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(10, bytesRead);
assertEquals("more stuff", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
}
public void testDecodingFromSessionBuffer() throws Exception {
@@ -153,22 +158,27 @@
assertEquals(6, bytesRead);
assertEquals("stuff;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(0, metrics.getBytesTransferred()); // doesn't count if from session buffer
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(10, bytesRead);
assertEquals("more stuff", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(10, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
-
+ assertEquals(10, metrics.getBytesTransferred());
+
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(10, metrics.getBytesTransferred());
+
}
public void testBasicDecodingFile() throws Exception {
@@ -194,6 +204,7 @@
}
}
+ assertEquals(testfile.length(), metrics.getBytesTransferred());
fchannel.close();
assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
@@ -227,12 +238,80 @@
}
}
+ // count everything except the initial 7 bytes that went to the session buffer
+ assertEquals(testfile.length() - 7, metrics.getBytesTransferred());
fchannel.close();
assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
fileHandle.delete();
}
+
+ public void testDecodingFileWithOffsetAndBufferedSessionData() throws Exception {
+ ReadableByteChannel channel = new ReadableByteChannelMockup(
+ new String[] {"stuff; ", "more stuff; ", "a lot more stuff!"}, "US-ASCII");
+ HttpParams params = new BasicHttpParams();
+
+ SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+ HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+ IdentityDecoder decoder = new IdentityDecoder(
+ channel, inbuf, metrics);
+
+ int i = inbuf.fill(channel);
+ assertEquals(7, i);
+
+ File fileHandle = File.createTempFile("testFile", ".txt");
+
+ RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
+ byte[] beginning = "beginning; ".getBytes("US-ASCII");
+ testfile.write(beginning);
+ testfile.close();
+
+ testfile = new RandomAccessFile(fileHandle, "rw");
+ FileChannel fchannel = testfile.getChannel();
+
+ long pos = beginning.length;
+ while (!decoder.isCompleted()) {
+ if(testfile.length() < pos)
+ testfile.setLength(pos);
+ long bytesRead = decoder.transfer(fchannel, pos, 10);
+ if (bytesRead > 0) {
+ pos += bytesRead;
+ }
+ }
+
+ // count everything except the initial 7 bytes that went to the session buffer
+ assertEquals(testfile.length() - 7 - beginning.length, metrics.getBytesTransferred());
+ fchannel.close();
+
+ assertEquals("beginning; stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
+
+ fileHandle.delete();
+ }
+
+ public void testWriteBeyondFileSize() throws Exception {
+ ReadableByteChannel channel = new ReadableByteChannelMockup(
+ new String[] {"a"}, "US-ASCII");
+ HttpParams params = new BasicHttpParams();
+
+ SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+ HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+ IdentityDecoder decoder = new IdentityDecoder(
+ channel, inbuf, metrics);
+
+ File fileHandle = File.createTempFile("testFile", ".txt");
+
+ RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
+ FileChannel fchannel = testfile.getChannel();
+ assertEquals(0, testfile.length());
+
+ try {
+ decoder.transfer(fchannel, 5, 10);
+ fail("expected IOException");
+ } catch(IOException iox) {}
+
+ fileHandle.delete();
+ }
public void testInvalidConstructor() {
ReadableByteChannel channel = new ReadableByteChannelMockup(
Modified: httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java?rev=665886&r1=665885&r2=665886&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java (original)
+++ httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java Mon Jun 9 13:47:42 2008
@@ -32,6 +32,7 @@
import java.io.File;
import java.io.FileInputStream;
+import java.io.IOException;
import java.io.InputStreamReader;
import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
@@ -116,17 +117,20 @@
assertEquals(6, bytesRead);
assertEquals("stuff;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(6, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(10, bytesRead);
assertEquals("more stuff", convert(dst));
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
}
public void testCodingBeyondContentLimit() throws Exception {
@@ -147,17 +151,20 @@
assertEquals(6, bytesRead);
assertEquals("stuff;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(6, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(10, bytesRead);
assertEquals("more stuff", convert(dst));
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
}
public void testBasicDecodingSmallBuffer() throws Exception {
@@ -176,35 +183,41 @@
assertEquals(4, bytesRead);
assertEquals("stuf", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(4, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(2, bytesRead);
assertEquals("f;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(6, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(4, bytesRead);
assertEquals("more", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(10, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(4, bytesRead);
assertEquals(" stu", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(14, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(2, bytesRead);
assertEquals("ff", convert(dst));
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
}
public void testDecodingFromSessionBuffer1() throws Exception {
@@ -228,17 +241,20 @@
assertEquals(6, bytesRead);
assertEquals("stuff;", convert(dst));
assertFalse(decoder.isCompleted());
+ assertEquals(0, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(10, bytesRead);
assertEquals("more stuff", convert(dst));
assertTrue(decoder.isCompleted());
+ assertEquals(10, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(10, metrics.getBytesTransferred());
}
public void testDecodingFromSessionBuffer2() throws Exception {
@@ -265,11 +281,13 @@
assertEquals(16, bytesRead);
assertEquals("stuff;more stuff", convert(dst));
assertTrue(decoder.isCompleted());
+ assertEquals(0, metrics.getBytesTransferred());
dst.clear();
bytesRead = decoder.read(dst);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(0, metrics.getBytesTransferred());
}
public void testBasicDecodingFile() throws Exception {
@@ -294,7 +312,7 @@
pos += bytesRead;
}
}
-
+ assertEquals(testfile.length(), metrics.getBytesTransferred());
fchannel.close();
assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
@@ -328,6 +346,7 @@
}
}
+ assertEquals(testfile.length() - 7, metrics.getBytesTransferred());
fchannel.close();
assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
@@ -335,6 +354,72 @@
fileHandle.delete();
}
+ public void testDecodingFileWithOffsetAndBufferedSessionData() throws Exception {
+ ReadableByteChannel channel = new ReadableByteChannelMockup(
+ new String[] {"stuff; ", "more stuff; ", "a lot more stuff!"}, "US-ASCII");
+ HttpParams params = new BasicHttpParams();
+
+ SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+ HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+ LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
+ channel, inbuf, metrics, 36);
+
+ int i = inbuf.fill(channel);
+ assertEquals(7, i);
+
+ File fileHandle = File.createTempFile("testFile", ".txt");
+
+ RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
+ byte[] beginning = "beginning; ".getBytes("US-ASCII");
+ testfile.write(beginning);
+ testfile.close();
+
+ testfile = new RandomAccessFile(fileHandle, "rw");
+ FileChannel fchannel = testfile.getChannel();
+
+ long pos = beginning.length;
+ while (!decoder.isCompleted()) {
+ if(testfile.length() < pos)
+ testfile.setLength(pos);
+ long bytesRead = decoder.transfer(fchannel, pos, 10);
+ if (bytesRead > 0) {
+ pos += bytesRead;
+ }
+ }
+
+ // count everything except the initial 7 bytes that went to the session buffer
+ assertEquals(testfile.length() - 7 - beginning.length, metrics.getBytesTransferred());
+ fchannel.close();
+
+ assertEquals("beginning; stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
+
+ fileHandle.delete();
+ }
+
+ public void testWriteBeyondFileSize() throws Exception {
+ ReadableByteChannel channel = new ReadableByteChannelMockup(
+ new String[] {"a"}, "US-ASCII");
+ HttpParams params = new BasicHttpParams();
+
+ SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+ HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+ LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
+ channel, inbuf, metrics, 1);
+
+ File fileHandle = File.createTempFile("testFile", ".txt");
+
+ RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
+ FileChannel fchannel = testfile.getChannel();
+ assertEquals(0, testfile.length());
+
+ try {
+ decoder.transfer(fchannel, 5, 10);
+ fail("expected IOException");
+ } catch(IOException iox) {}
+
+ fileHandle.delete();
+ }
+
public void testCodingBeyondContentLimitFile() throws Exception {
ReadableByteChannel channel = new ReadableByteChannelMockup(
new String[] {
@@ -354,14 +439,17 @@
long bytesRead = decoder.transfer(fchannel, 0, 6);
assertEquals(6, bytesRead);
assertFalse(decoder.isCompleted());
+ assertEquals(6, metrics.getBytesTransferred());
bytesRead = decoder.transfer(fchannel,0 , 10);
assertEquals(10, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
bytesRead = decoder.transfer(fchannel, 0, 1);
assertEquals(-1, bytesRead);
assertTrue(decoder.isCompleted());
+ assertEquals(16, metrics.getBytesTransferred());
fileHandle.delete();
}