Merge pull request iipc#39 from nlevitt/issue-38

fix for iipc#38 - detect end of http protocol headers in a smarter way, to avoid calling write(byte) repeatedly; add unit tests

Author: Kristinn Sigurðsson

CHANGES.md

@@ -3,6 +3,7 @@
 * [Escape redirect URLs in RealCDXExtractorOutput](https://github.com/iipc/webarchive-commons/pull/36)
 * [Tests fail on Windows](https://github.com/iipc/webarchive-commons/issues/2)
 * [Test fails on Java 8](https://github.com/iipc/webarchive-commons/issues/31)
+* [RecordingOutputStream can affect tcp packets sent in an undesirable way](https://github.com/iipc/webarchive-commons/issues/38)
 
 1.1.4
 -----

pom.xml

@@ -65,7 +65,6 @@
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
       <version>3.8.1</version>
-      <scope>test</scope>
     </dependency>
 
     <dependency>

src/main/java/org/archive/io/RecordingOutputStream.java

@@ -242,6 +242,26 @@ public void write(int b) throws IOException {
         checkLimits();
     }
 
+    private int findMessageBodyBeginMark(byte[] b, int off, int len) {
+        if ((lastTwoBytes[1] == '\n' || lastTwoBytes[0] == '\n' && lastTwoBytes[1] == '\r')
+                && len >= 1 && b[off] == '\n') {
+            return 1;
+        } else if (lastTwoBytes[1] == '\n' && len >= 2 && b[off] == '\r' && b[off+1] == '\n') {
+            return 2;
+        }
+
+        for (int i = off; i < off + len - 1; i++) {
+            if (b[i] == '\n' && b[i+1] == '\n') {
+                return i + 2;
+            } else if (b[i] == '\n' && b[i+1] == '\r'
+                    && i + 2 < off + len && b[i+2] == '\n') {
+                return i + 3;
+            }
+        }
+
+        return -1;
+    }
+
     public void write(byte[] b, int off, int len) throws IOException {
         if(position < maxPosition) {
             if(position+len<=maxPosition) {
@@ -255,20 +275,35 @@ public void write(byte[] b, int off, int len) throws IOException {
             off += consumeRange;
             len -= consumeRange; 
         }
-        
-        // see comment on int[] lastTwoBytes
-        while (messageBodyBeginMark < 0 && len > 0) {
-            write(b[off]);
-            off++;
-            len--;
+
+        if (messageBodyBeginMark < 0) {
+            // see comment on int[] lastTwoBytes
+            int mark = findMessageBodyBeginMark(b, off, len);
+            if (mark > 0) {
+                if(recording) {
+                    record(b, off, mark - off);
+                }
+                if (this.out != null) {
+                    this.out.write(b, off, mark - off);
+                }
+                markMessageBodyBegin();
+                len = len - (mark - off);
+                off = mark;
+            }
         }
-        
+
         if(recording) {
             record(b, off, len);
         }
         if (this.out != null) {
             this.out.write(b, off, len);
         }
+        if (len >= 1) {
+            lastTwoBytes[1] = b[off + len - 1];
+            if (len >= 2) {
+                lastTwoBytes[0] = b[off + len - 2];
+            }
+        }
         checkLimits();
     }
 

src/main/java/org/archive/util/TmpDirTestCase.java

@@ -0,0 +1,119 @@
+/*
+ *  This file is part of the Heritrix web crawler (crawler.archive.org).
+ *
+ *  Licensed to the Internet Archive (IA) by one or more individual 
+ *  contributors. 
+ *
+ *  The IA licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.archive.util;
+
+import java.io.File;
+import java.io.IOException;
+
+import junit.framework.TestCase;
+
+
+/**
+ * Base class for TestCases that want access to a tmp dir for the writing
+ * of files.
+ *
+ * @author stack
+ */
+public abstract class TmpDirTestCase extends TestCase
+{
+    /**
+     * Name of the system property that holds pointer to tmp directory into
+     * which we can safely write files.
+     */
+    public static final String TEST_TMP_SYSTEM_PROPERTY_NAME = "testtmpdir";
+
+    /**
+     * Default test tmp.
+     */
+    public static final String DEFAULT_TEST_TMP_DIR = File.separator + "tmp" +
+        File.separator + "heritrix-junit-tests";
+
+    /**
+     * Directory to write temporary files to.
+     */
+    private File tmpDir = null;
+
+
+    public TmpDirTestCase()
+    {
+        super();
+    }
+
+    public TmpDirTestCase(String testName)
+    {
+        super(testName);
+    }
+
+    /*
+     * @see TestCase#setUp()
+     */
+    protected void setUp() throws Exception {
+        super.setUp();
+        this.tmpDir = tmpDir();
+    }
+
+    /**
+     * @return Returns the tmpDir.
+     */
+    public File getTmpDir()
+    {
+        return this.tmpDir;
+    }
+
+    /**
+     * Delete any files left over from previous run.
+     *
+     * @param basename Base name of files we're to clean up.
+     */
+    public void cleanUpOldFiles(String basename) {
+        cleanUpOldFiles(getTmpDir(), basename);
+    }
+
+    /**
+     * Delete any files left over from previous run.
+     *
+     * @param prefix Base name of files we're to clean up.
+     * @param basedir Directory to start cleaning in.
+     */
+    public void cleanUpOldFiles(File basedir, String prefix) {
+        File [] files = FileUtils.getFilesWithPrefix(basedir, prefix);
+        if (files != null) {
+            for (int i = 0; i < files.length; i++) {
+                org.apache.commons.io.FileUtils.deleteQuietly(files[i]);
+            }
+        }
+    }
+    
+    
+    public static File tmpDir() throws IOException {
+        String tmpDirStr = System.getProperty(TEST_TMP_SYSTEM_PROPERTY_NAME);
+        tmpDirStr = (tmpDirStr == null)? DEFAULT_TEST_TMP_DIR: tmpDirStr;
+        File tmpDir = new File(tmpDirStr);
+        FileUtils.ensureWriteableDirectory(tmpDir);
+
+        if (!tmpDir.canWrite())
+        {
+            throw new IOException(tmpDir.getAbsolutePath() +
+                 " is unwriteable.");
+        }
+        
+        return tmpDir;
+    }
+}