Bugfix: The lax parser for HTTP-status did not accept ":" in the line, but ":" is legal according to RFC2616

tokee · tokee · commit 6f5c60734d49 · 2015-02-18T14:45:33.000+01:00
Test: Extended unit test to check for extracted status
diff --git a/src/main/java/org/archive/io/arc/ARCRecord.java b/src/main/java/org/archive/io/arc/ARCRecord.java
@@ -596,11 +596,12 @@ private InputStream readHttpHeader() throws IOException {
         	
         	// If it's actually the status line, break, otherwise continue skipping any
         	// previous header values
-        	if (!statusLine.contains(":") && StatusLine.startsWithHTTP(statusLine)) {
+            // Old code contained {@code !statusLine.contains(":")}, which conflicts with RFC2616-sec6
+        	if (StatusLine.startsWithHTTP(statusLine)) {
         		break;
         	}
 
-            if (statusLine.replace("\r", "").isEmpty()) { // No more headerlines
+            if (statusLine.replace("\r", "").isEmpty()) { // No more header lines
                 break;
             }
 
diff --git a/src/test/java/org/archive/io/arc/ARCReaderFactoryTest.java b/src/test/java/org/archive/io/arc/ARCReaderFactoryTest.java
@@ -57,13 +57,13 @@ private void offsetResourceTest( File testfile, long offset, String uri ) throws
     }
 
     public void testBaseSampleARC() throws IOException {
-        testARCReaderIteration(testfile1, 9);
+        testARCReaderIteration(testfile1, 9, 7);
     }
     /*
     This failed with the old http-header parsing code in {@code ARCRecord#readHttpHeader}.
      */
     public void testNewlinedSampleARC() throws IOException {
-        testARCReaderIteration(testfile_nl, 4);
+        testARCReaderIteration(testfile_nl, 4, 3); // Status has 2*200 & 1*404
     }
 
     // Independent of the ARCReader code
@@ -88,10 +88,14 @@ public void testBaseSampleARCContentLength() throws IOException {
 //    }
 
     // Uncomment println for manual inspection of first content line
-    private void testARCReaderIteration(File arc, int expectedRecords) throws IOException {
+    private void testARCReaderIteration(File arc, int expectedRecords, int hasStatus) throws IOException {
         ARCReader reader = ARCReaderFactory.get(arc);
         int recordCount = 0;
+        int okCount = 0;
         for (ArchiveRecord record : reader) {
+            if (((ARCRecord)record).getStatusCode() != -1) {
+                okCount++;
+            }
             SubInputStream sub = new SubInputStream(record);
             sub.skip(record.getHeader().getContentBegin());
             //System.out.println(record.getPosition() + "> " + sub.readLine());
@@ -100,6 +104,7 @@ private void testARCReaderIteration(File arc, int expectedRecords) throws IOExce
         }
         reader.close();
         assertEquals("There should be the right number of records in " + arc, expectedRecords, recordCount);
+        assertEquals("There should be the right number of status 200 records in " + arc, hasStatus, okCount);
     }
 
     private static File getResource(String resource) {

Original file line number	Diff line number	Diff line change
`@@ -596,11 +596,12 @@ private InputStream readHttpHeader() throws IOException {`
`596`	`596`
`597`	`597`	`// If it's actually the status line, break, otherwise continue skipping any`
`598`	`598`	`// previous header values`
`599`		`- if (!statusLine.contains(":") && StatusLine.startsWithHTTP(statusLine)) {`
	`599`	`+ // Old code contained {@code !statusLine.contains(":")}, which conflicts with RFC2616-sec6`
	`600`	`+ if (StatusLine.startsWithHTTP(statusLine)) {`
`600`	`601`	`break;`
`601`	`602`	`}`
`602`	`603`
`603`		`- if (statusLine.replace("\r", "").isEmpty()) { // No more headerlines`
	`604`	`+ if (statusLine.replace("\r", "").isEmpty()) { // No more header lines`
`604`	`605`	`break;`
`605`	`606`	`}`
`606`	`607`