hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1102241 - in /httpcomponents/httpcore/trunk: ./ httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/ httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/ httpcore/src/main/java/org/apache/http/impl/io/ httpcore/src/test/java/...
Date Thu, 12 May 2011 11:16:57 GMT
Author: olegk
Date: Thu May 12 11:16:56 2011
New Revision: 1102241

URL: http://svn.apache.org/viewvc?rev=1102241&view=rev
Log:
In case of an unexpected end of stream condition (the peer closed connection prematurely)
truncated Content-Length delimited message bodies to cause an I/O exception

Modified:
    httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
    httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
    httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
    httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/io/ContentLengthInputStream.java
    httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/io/TestContentLengthInputStream.java

Modified: httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?rev=1102241&r1=1102240&r2=1102241&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Thu May 12 11:16:56 2011
@@ -1,4 +1,11 @@
 Changes since 4.1
+-------------------
+
+* In case of an unexpected end of stream condition (the peer closed connection prematurely)

+  truncated Content-Length delimited message bodies will cause an I/O exception. Application
+  can still choose to catch and ignore ConnectionClosedException in order to accept partial

+  message content.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
 
 * [HTTPCORE-255]: Fixed resource management in InputStreamEntity#writeTo()
   Contributed by Oleg Kalnichevski <olegk at apache.org>

Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java?rev=1102241&r1=1102240&r2=1102241&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/impl/nio/codecs/LengthDelimitedDecoder.java
Thu May 12 11:16:56 2011
@@ -32,6 +32,7 @@ import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
 
+import org.apache.http.ConnectionClosedException;
 import org.apache.http.impl.io.HttpTransportMetricsImpl;
 import org.apache.http.nio.FileContentDecoder;
 import org.apache.http.nio.reactor.SessionInputBuffer;
@@ -97,7 +98,11 @@ public class LengthDelimitedDecoder exte
         }
         if (bytesRead == -1) {
             this.completed = true;
-            return -1;
+            if (this.len < this.contentLength) {
+                throw new ConnectionClosedException(
+                        "Premature end of Content-Length delimited message body (expected:
"
+                        + this.contentLength + "; received: " + this.len);
+            }
         }
         this.len += bytesRead;
         if (this.len >= this.contentLength) {
@@ -149,7 +154,11 @@ public class LengthDelimitedDecoder exte
         }
         if (bytesRead == -1) {
             this.completed = true;
-            return -1;
+            if (this.len < this.contentLength) {
+                throw new ConnectionClosedException(
+                        "Premature end of Content-Length delimited message body (expected:
"
+                        + this.contentLength + "; received: " + this.len);
+            }
         }
         this.len += bytesRead;
         if (this.len >= this.contentLength) {

Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java?rev=1102241&r1=1102240&r2=1102241&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java
Thu May 12 11:16:56 2011
@@ -36,12 +36,14 @@ import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
 
+import org.apache.http.ConnectionClosedException;
 import org.apache.http.ReadableByteChannelMock;
 import org.apache.http.impl.io.HttpTransportMetricsImpl;
 import org.apache.http.impl.nio.reactor.SessionInputBufferImpl;
 import org.apache.http.nio.reactor.SessionInputBuffer;
 import org.apache.http.params.BasicHttpParams;
 import org.apache.http.params.HttpParams;
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -50,6 +52,20 @@ import org.junit.Test;
  */
 public class TestLengthDelimitedDecoder {
 
+    private File tmpfile;
+
+    protected File createTempFile() throws IOException {
+        this.tmpfile = File.createTempFile("testFile", ".txt");
+        return this.tmpfile;
+    }
+
+    @After
+    public void deleteTempFile() {
+        if (this.tmpfile != null && this.tmpfile.exists()) {
+            this.tmpfile.delete();
+        }
+    }
+
     private static String convert(final ByteBuffer src) {
         src.flip();
         StringBuilder buffer = new StringBuilder(src.remaining());
@@ -280,30 +296,22 @@ public class TestLengthDelimitedDecoder 
         LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
                 channel, inbuf, metrics, 36);
 
-        File fileHandle = File.createTempFile("testFile", ".txt");
-
-        RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
-        FileChannel fchannel = testfile.getChannel();
-
-        long pos = 0;
-        while (!decoder.isCompleted()) {
-            long bytesRead = decoder.transfer(fchannel, pos, 10);
-            if (bytesRead > 0) {
-                pos += bytesRead;
+        createTempFile();
+        RandomAccessFile testfile = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            FileChannel fchannel = testfile.getChannel();
+            long pos = 0;
+            while (!decoder.isCompleted()) {
+                long bytesRead = decoder.transfer(fchannel, pos, 10);
+                if (bytesRead > 0) {
+                    pos += bytesRead;
+                }
             }
+        } finally {
+            testfile.close();
         }
-        Assert.assertEquals(testfile.length(), metrics.getBytesTransferred());
-        fchannel.close();
-
-        Assert.assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
-
-        deleteWithCheck(fileHandle);
-    }
-
-    private void deleteWithCheck(File handle){
-        if (!handle.delete() && handle.exists()){
-            System.err.println("Failed to delete: "+handle.getPath());
-        }
+        Assert.assertEquals(this.tmpfile.length(), metrics.getBytesTransferred());
+        Assert.assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(this.tmpfile));
     }
 
     @Test
@@ -320,25 +328,22 @@ public class TestLengthDelimitedDecoder 
         int i = inbuf.fill(channel);
         Assert.assertEquals(7, i);
 
-        File fileHandle = File.createTempFile("testFile", ".txt");
-
-        RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
-        FileChannel fchannel = testfile.getChannel();
-
-        long pos = 0;
-        while (!decoder.isCompleted()) {
-            long bytesRead = decoder.transfer(fchannel, pos, 10);
-            if (bytesRead > 0) {
-                pos += bytesRead;
+        createTempFile();
+        RandomAccessFile testfile = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            FileChannel fchannel = testfile.getChannel();
+            long pos = 0;
+            while (!decoder.isCompleted()) {
+                long bytesRead = decoder.transfer(fchannel, pos, 10);
+                if (bytesRead > 0) {
+                    pos += bytesRead;
+                }
             }
+        } finally {
+            testfile.close();
         }
-
-        Assert.assertEquals(testfile.length() - 7, metrics.getBytesTransferred());
-        fchannel.close();
-
-        Assert.assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
-
-        deleteWithCheck(fileHandle);
+        Assert.assertEquals(this.tmpfile.length() - 7, metrics.getBytesTransferred());
+        Assert.assertEquals("stuff; more stuff; a lot more stuff!", readFromFile(this.tmpfile));
     }
 
     @Test
@@ -355,33 +360,36 @@ public class TestLengthDelimitedDecoder 
         int i = inbuf.fill(channel);
         Assert.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();
+        createTempFile();
+        RandomAccessFile testfile = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            testfile.write(beginning);
+        } finally {
+            testfile.close();
+        }
+
+        testfile = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            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;
+            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;
+                }
             }
+        } finally {
+            testfile.close();
         }
 
         // count everything except the initial 7 bytes that went to the session buffer
-        Assert.assertEquals(testfile.length() - 7 - beginning.length, metrics.getBytesTransferred());
-        fchannel.close();
-
-        Assert.assertEquals("beginning; stuff; more stuff; a lot more stuff!", readFromFile(fileHandle));
-
-        deleteWithCheck(fileHandle);
+        Assert.assertEquals(this.tmpfile.length() - 7 - beginning.length, metrics.getBytesTransferred());
+        Assert.assertEquals("beginning; stuff; more stuff; a lot more stuff!", readFromFile(this.tmpfile));
     }
 
     @Test
@@ -395,19 +403,19 @@ public class TestLengthDelimitedDecoder 
         LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
                 channel, inbuf, metrics, 1);
 
-        File fileHandle = File.createTempFile("testFile", ".txt");
-
-        RandomAccessFile testfile = new RandomAccessFile(fileHandle, "rw");
-        FileChannel fchannel = testfile.getChannel();
-        Assert.assertEquals(0, testfile.length());
-
+        createTempFile();
+        RandomAccessFile testfile = new RandomAccessFile(this.tmpfile, "rw");
         try {
-            decoder.transfer(fchannel, 5, 10);
-            Assert.fail("expected IOException");
-        } catch(IOException iox) {}
-
-        testfile.close();
-        deleteWithCheck(fileHandle);
+            FileChannel fchannel = testfile.getChannel();
+            Assert.assertEquals(0, testfile.length());
+            try {
+                decoder.transfer(fchannel, 5, 10);
+                Assert.fail("IOException should have been thrown");
+            } catch(IOException expected) {
+            }
+        } finally {
+            testfile.close();
+        }
     }
 
     @Test
@@ -423,27 +431,28 @@ public class TestLengthDelimitedDecoder 
         LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
                 channel, inbuf, metrics, 16);
 
-        File fileHandle = File.createTempFile("testFile", ".txt");
-        RandomAccessFile testfile  = new RandomAccessFile(fileHandle, "rw");
-        FileChannel fchannel = testfile.getChannel();
-
-        long bytesRead = decoder.transfer(fchannel, 0, 6);
-        Assert.assertEquals(6, bytesRead);
-        Assert.assertFalse(decoder.isCompleted());
-        Assert.assertEquals(6, metrics.getBytesTransferred());
-
-        bytesRead = decoder.transfer(fchannel,0 , 10);
-        Assert.assertEquals(10, bytesRead);
-        Assert.assertTrue(decoder.isCompleted());
-        Assert.assertEquals(16, metrics.getBytesTransferred());
-
-        bytesRead = decoder.transfer(fchannel, 0, 1);
-        Assert.assertEquals(-1, bytesRead);
-        Assert.assertTrue(decoder.isCompleted());
-        Assert.assertEquals(16, metrics.getBytesTransferred());
+        createTempFile();
+        RandomAccessFile testfile  = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            FileChannel fchannel = testfile.getChannel();
 
-        testfile.close();
-        deleteWithCheck(fileHandle);
+            long bytesRead = decoder.transfer(fchannel, 0, 6);
+            Assert.assertEquals(6, bytesRead);
+            Assert.assertFalse(decoder.isCompleted());
+            Assert.assertEquals(6, metrics.getBytesTransferred());
+
+            bytesRead = decoder.transfer(fchannel,0 , 10);
+            Assert.assertEquals(10, bytesRead);
+            Assert.assertTrue(decoder.isCompleted());
+            Assert.assertEquals(16, metrics.getBytesTransferred());
+
+            bytesRead = decoder.transfer(fchannel, 0, 1);
+            Assert.assertEquals(-1, bytesRead);
+            Assert.assertTrue(decoder.isCompleted());
+            Assert.assertEquals(16, metrics.getBytesTransferred());
+        } finally {
+            testfile.close();
+        }
     }
 
     @Test
@@ -519,4 +528,45 @@ public class TestLengthDelimitedDecoder 
         Assert.assertEquals(0, metrics.getBytesTransferred());
     }
 
+    @Test(expected=ConnectionClosedException.class)
+    public void testTruncatedContent() throws Exception {
+        ReadableByteChannel channel = new ReadableByteChannelMock(
+                new String[] {"1234567890"}, "US-ASCII");
+        HttpParams params = new BasicHttpParams();
+
+        SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+        HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+        LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
+                channel, inbuf, metrics, 20);
+
+        ByteBuffer dst = ByteBuffer.allocate(1024);
+
+        int bytesRead = decoder.read(dst);
+        Assert.assertEquals(10, bytesRead);
+        decoder.read(dst);
+    }
+
+    @Test(expected=ConnectionClosedException.class)
+    public void testTruncatedContentWithFile() throws Exception {
+        ReadableByteChannel channel = new ReadableByteChannelMock(
+                new String[] {"1234567890"}, "US-ASCII");
+        HttpParams params = new BasicHttpParams();
+
+        SessionInputBuffer inbuf = new SessionInputBufferImpl(1024, 256, params);
+        HttpTransportMetricsImpl metrics = new HttpTransportMetricsImpl();
+        LengthDelimitedDecoder decoder = new LengthDelimitedDecoder(
+                channel, inbuf, metrics, 20);
+
+        createTempFile();
+        RandomAccessFile testfile  = new RandomAccessFile(this.tmpfile, "rw");
+        try {
+            FileChannel fchannel = testfile.getChannel();
+            long bytesRead = decoder.transfer(fchannel, 0, Integer.MAX_VALUE);
+            Assert.assertEquals(10, bytesRead);
+            decoder.transfer(fchannel, 0, Integer.MAX_VALUE);
+        } finally {
+            testfile.close();
+        }
+    }
+
 }

Modified: httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/io/ContentLengthInputStream.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/io/ContentLengthInputStream.java?rev=1102241&r1=1102240&r2=1102241&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/io/ContentLengthInputStream.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/io/ContentLengthInputStream.java
Thu May 12 11:16:56 2011
@@ -30,6 +30,7 @@ package org.apache.http.impl.io;
 import java.io.IOException;
 import java.io.InputStream;
 
+import org.apache.http.ConnectionClosedException;
 import org.apache.http.io.BufferInfo;
 import org.apache.http.io.SessionInputBuffer;
 
@@ -100,8 +101,10 @@ public class ContentLengthInputStream ex
     public void close() throws IOException {
         if (!closed) {
             try {
-                byte buffer[] = new byte[BUFFER_SIZE];
-                while (read(buffer) >= 0) {
+                if (pos < contentLength) {
+                    byte buffer[] = new byte[BUFFER_SIZE];
+                    while (read(buffer) >= 0) {
+                    }
                 }
             } finally {
                 // close after above so that we don't throw an exception trying
@@ -136,8 +139,17 @@ public class ContentLengthInputStream ex
         if (pos >= contentLength) {
             return -1;
         }
-        pos++;
-        return this.in.read();
+        int b = this.in.read();
+        if (b == -1) {
+            if (pos < contentLength) {
+                throw new ConnectionClosedException(
+                        "Premature end of Content-Length delimited message body (expected:
"
+                        + contentLength + "; received: " + pos);
+            }
+        } else {
+            pos++;
+        }
+        return b;
     }
 
     /**
@@ -166,7 +178,14 @@ public class ContentLengthInputStream ex
             len = (int) (contentLength - pos);
         }
         int count = this.in.read(b, off, len);
-        pos += count;
+        if (count == -1 && pos < contentLength) {
+            throw new ConnectionClosedException(
+                    "Premature end of Content-Length delimited message body (expected: "
+                    + contentLength + "; received: " + pos);
+        }
+        if (count > 0) {
+            pos += count;
+        }
         return count;
     }
 

Modified: httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/io/TestContentLengthInputStream.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/io/TestContentLengthInputStream.java?rev=1102241&r1=1102240&r2=1102241&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/io/TestContentLengthInputStream.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/io/TestContentLengthInputStream.java
Thu May 12 11:16:56 2011
@@ -31,7 +31,9 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
+import org.apache.http.ConnectionClosedException;
 import org.apache.http.impl.SessionInputBufferMock;
+import org.apache.http.io.SessionInputBuffer;
 import org.apache.http.util.EncodingUtils;
 import org.junit.Assert;
 import org.junit.Test;
@@ -92,10 +94,6 @@ public class TestContentLengthInputStrea
         Assert.assertTrue(in.skip(-1) == 0);
         Assert.assertTrue(in.read() == -1);
 
-        in = new ContentLengthInputStream(new SessionInputBufferMock(new byte[2]), 4L);
-        in.read();
-        Assert.assertTrue(in.skip(2) == 1);
-
         in = new ContentLengthInputStream(new SessionInputBufferMock(new byte[20]), 10L);
         Assert.assertEquals(5,in.skip(5));
         Assert.assertEquals(5, in.read(new byte[20]));
@@ -112,9 +110,10 @@ public class TestContentLengthInputStrea
 
     @Test
     public void testClose() throws IOException {
-        String correct = "1234567890123456";
-        InputStream in = new ContentLengthInputStream(new SessionInputBufferMock(
-            EncodingUtils.getBytes(correct, CONTENT_CHARSET)), 10L);
+        String correct = "1234567890123456-";
+        SessionInputBuffer inbuffer = new SessionInputBufferMock(EncodingUtils.getBytes(
+                correct, CONTENT_CHARSET));
+        InputStream in = new ContentLengthInputStream(inbuffer, 16L);
         in.close();
         in.close();
         try {
@@ -136,6 +135,28 @@ public class TestContentLengthInputStrea
         } catch (IOException ex) {
             // expected
         }
+        Assert.assertEquals('-', inbuffer.read());
+    }
+
+    @Test
+    public void testTruncatedContent() throws IOException {
+        String correct = "1234567890123456";
+        SessionInputBuffer inbuffer = new SessionInputBufferMock(EncodingUtils.getBytes(
+                correct, CONTENT_CHARSET));
+        InputStream in = new ContentLengthInputStream(inbuffer, 32L);
+        byte[] tmp = new byte[32];
+        int byteRead = in.read(tmp);
+        Assert.assertEquals(16, byteRead);
+        try {
+            in.read(tmp);
+            Assert.fail("ConnectionClosedException should have been closed");
+        } catch (ConnectionClosedException ex) {
+        }
+        try {
+            in.read();
+            Assert.fail("ConnectionClosedException should have been closed");
+        } catch (ConnectionClosedException ex) {
+        }
     }
 
 }



Mime
View raw message