hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sber...@apache.org
Subject 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/
Date Mon, 09 Jun 2008 20:47:43 GMT
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();
     }



Mime
View raw message