From afa88c025e4b7c195637cf4d1d15fde931e2a6e5 Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Mon, 11 Mar 2019 21:35:13 +0000 Subject: [PATCH 01/13] Ensure we start parsing full lines, for #239. --- .../main/java/org/archive/crawler/framework/CrawlJob.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java b/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java index 0718c292..c9aa8bb8 100644 --- a/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java +++ b/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java @@ -172,6 +172,12 @@ public class CrawlJob implements Comparable, ApplicationListener Date: Sat, 16 Mar 2019 11:50:53 +0900 Subject: [PATCH 02/13] Handle commas more compliantly when parsing srcset Commas are allowed if they're in the middle of the URL. Consequently: srcset="a,b,,c," => ["a,b,,c"] srcset="a, b,, c," => ["a", "b", "c"] They occur particularly commonly in data: URLs before the base64 value. Commas are also allowed in descriptors if they are enclosed by parens: srcset="a (b,c),d" => ["a", "d"] Spec: https://html.spec.whatwg.org/multipage/images.html#parsing-a-srcset-attribute --- .../java/org/archive/modules/extractor/ExtractorHTML.java | 8 +++++--- .../org/archive/modules/extractor/ExtractorHTMLTest.java | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java b/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java index 54d5870d..3cdae15c 100644 --- a/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java +++ b/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java @@ -682,13 +682,15 @@ public class ExtractorHTML extends ContentExtractor implements InitializingBean logger.fine("Found srcset listing: " + value.toString()); - String[] links = value.toString().split(","); - for (int i=0; i < links.length; i++){ - String link = links[i].trim().split(" +")[0]; + Matcher matcher = TextUtils.getMatcher("[\\s,]*(\\S*[^,\\s])(?:\\s(?:[^,(]+|\\([^)]*\\))*)?", value); + while (matcher.lookingAt()) { + String link = matcher.group(1); + matcher.region(matcher.end(), matcher.regionEnd()); logger.finer("Found " + link + " adding to outlinks."); addLinkFromString(curi, link, context, hop); numberOfLinksExtracted.incrementAndGet(); } + TextUtils.recycleMatcher(matcher); } else { addLinkFromString(curi, (value instanceof String)? diff --git a/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java b/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java index 5560a619..7327d22b 100644 --- a/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java +++ b/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java @@ -512,7 +512,7 @@ public class ExtractorHTMLTest extends StringExtractorTestBase { CharSequence cs = "\"\""; getExtractor().extract(curi, cs); @@ -521,6 +521,8 @@ public class ExtractorHTMLTest extends StringExtractorTestBase { Arrays.sort(links); String[] dest = { + "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7", + "http://www.example.com/a,b,c", "http://www.example.com/images/foo.jpg", "http://www.example.com/images/foo1.jpg", "http://www.example.com/images/foo2.jpg", From e1d93e3308adfe07ed10d75c6503087aed99383c Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Sat, 16 Mar 2019 12:35:57 +0900 Subject: [PATCH 03/13] Don't run srcset test against jericho, it doesn't handle it --- .../modules/extractor/JerichoExtractorHTMLTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java b/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java index 536c7d7c..a0c612c7 100644 --- a/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java +++ b/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java @@ -152,6 +152,15 @@ public class JerichoExtractorHTMLTest extends ExtractorHTMLTest { public void testConditionalComment1() throws URIException { } + /* + * Override of ExtractorHTMLTest method because the test fails with + * JerichoExtractorHTML + */ + @Override + public void testImgSrcSetAttribute() throws URIException { + // jericho parser doesn't understand srcset + } + /* * Override of ExtractorHTMLTest method because the test fails with * JerichoExtractorHTML From 2a34fcffd6ef18b48c05282d8237cd16ac7685b2 Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Sat, 16 Mar 2019 15:47:50 +0900 Subject: [PATCH 04/13] Teach jericho extractor srcset --- .../archive/modules/extractor/JerichoExtractorHTML.java | 7 +++++++ .../modules/extractor/JerichoExtractorHTMLTest.java | 9 --------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/modules/src/main/java/org/archive/modules/extractor/JerichoExtractorHTML.java b/modules/src/main/java/org/archive/modules/extractor/JerichoExtractorHTML.java index 77e84f4a..c12d91ed 100644 --- a/modules/src/main/java/org/archive/modules/extractor/JerichoExtractorHTML.java +++ b/modules/src/main/java/org/archive/modules/extractor/JerichoExtractorHTML.java @@ -196,6 +196,13 @@ public class JerichoExtractorHTML extends ExtractorHTML { processEmbed(curi, attrValue, context, hopType); } + // SRCSET + if (((attr = attributes.get("srcset")) != null) && + ((attrValue = attr.getValue()) != null)) { + codebase = StringEscapeUtils.unescapeHtml(attrValue); + CharSequence context = elementContext(elementName, attr.getKey()); + processEmbed(curi, codebase, context); + } // CODEBASE if (((attr = attributes.get("codebase")) != null) && ((attrValue = attr.getValue()) != null)) { diff --git a/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java b/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java index a0c612c7..d90a9173 100644 --- a/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java +++ b/modules/src/test/java/org/archive/modules/extractor/JerichoExtractorHTMLTest.java @@ -151,15 +151,6 @@ public class JerichoExtractorHTMLTest extends ExtractorHTMLTest { @Override public void testConditionalComment1() throws URIException { } - - /* - * Override of ExtractorHTMLTest method because the test fails with - * JerichoExtractorHTML - */ - @Override - public void testImgSrcSetAttribute() throws URIException { - // jericho parser doesn't understand srcset - } /* * Override of ExtractorHTMLTest method because the test fails with From 7d91a1d4ee51c5dfc9b3670094c89e2ed9ce9bba Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Sat, 16 Mar 2019 16:02:48 +0900 Subject: [PATCH 05/13] Handle missing closing paren in srcset descriptor --- .../main/java/org/archive/modules/extractor/ExtractorHTML.java | 2 +- .../java/org/archive/modules/extractor/ExtractorHTMLTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java b/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java index 3cdae15c..c0595940 100644 --- a/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java +++ b/modules/src/main/java/org/archive/modules/extractor/ExtractorHTML.java @@ -682,7 +682,7 @@ public class ExtractorHTML extends ContentExtractor implements InitializingBean logger.fine("Found srcset listing: " + value.toString()); - Matcher matcher = TextUtils.getMatcher("[\\s,]*(\\S*[^,\\s])(?:\\s(?:[^,(]+|\\([^)]*\\))*)?", value); + Matcher matcher = TextUtils.getMatcher("[\\s,]*(\\S*[^,\\s])(?:\\s(?:[^,(]+|\\([^)]*(?:\\)|$))*)?", value); while (matcher.lookingAt()) { String link = matcher.group(1); matcher.region(matcher.end(), matcher.regionEnd()); diff --git a/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java b/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java index 7327d22b..3513ff11 100644 --- a/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java +++ b/modules/src/test/java/org/archive/modules/extractor/ExtractorHTMLTest.java @@ -512,7 +512,7 @@ public class ExtractorHTMLTest extends StringExtractorTestBase { CharSequence cs = "\"\""; getExtractor().extract(curi, cs); From 102b5086eb8174241ceb9f87861e373cd4601b4a Mon Sep 17 00:00:00 2001 From: Mike Izbicki Date: Mon, 18 Mar 2019 20:26:15 -0700 Subject: [PATCH 06/13] Update README.md Fix typo in link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 15c37c56..5f307433 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ you or adapt their server behavior accordingly. ## 3. Getting Started -See the User Manual, available from ## 4. Developer Documentation From 0235a0e5531b245c1ec80b319955fbd041c03246 Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Wed, 20 Mar 2019 20:26:51 +0000 Subject: [PATCH 07/13] Updated POM to use latest version. --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 033cd488..7df433e1 100644 --- a/pom.xml +++ b/pom.xml @@ -127,12 +127,12 @@ http://maven.apache.org/guides/mini/guide-m1-m2.html org.apache.httpcomponents httpclient - 4.3.6 + 4.5.7 org.apache.httpcomponents httpmime - 4.3.6 + 4.5.7 From e910bb6a5b9812c63add200bce07233730d859ef Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Wed, 20 Mar 2019 21:18:19 +0000 Subject: [PATCH 08/13] Supply an iterator, for #245 --- .../main/java/org/archive/modules/fetcher/BdbCookieStore.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java b/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java index 0f047bbc..d900363d 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java +++ b/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java @@ -20,6 +20,7 @@ package org.archive.modules.fetcher; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.Iterator; @@ -78,7 +79,7 @@ public class BdbCookieStore extends AbstractCookieStore implements @Override public int size() { return wrapped.size(); } @Override public boolean isEmpty() { throw new RuntimeException("not implemented"); } @Override public boolean contains(Object o) { throw new RuntimeException("not implemented"); } - @Override public Iterator iterator() { throw new RuntimeException("not implemented"); } + @Override public Iterator iterator() { return (Iterator) Arrays.asList(wrapped.toArray()).iterator(); } @Override public Object[] toArray() { return wrapped.toArray(); } @SuppressWarnings("hiding") @Override public T[] toArray(T[] a) { return wrapped.toArray(a); } @Override public boolean add(T e) { throw new RuntimeException("immutable list"); } From 2fbe60387864c1ba730daadf84b42db23b32340f Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Wed, 20 Mar 2019 22:02:35 +0000 Subject: [PATCH 09/13] Avoid deprecated flag. --- .../main/java/org/archive/modules/fetcher/FetchHTTPRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java index 3ee303ee..530eb943 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java +++ b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java @@ -400,7 +400,7 @@ public class FetchHTTPRequest { if (fetcher.getIgnoreCookies()) { requestConfigBuilder.setCookieSpec(CookieSpecs.IGNORE_COOKIES); } else { - requestConfigBuilder.setCookieSpec(CookieSpecs.BROWSER_COMPATIBILITY); + requestConfigBuilder.setCookieSpec(CookieSpecs.DEFAULT); } requestConfigBuilder.setConnectionRequestTimeout(fetcher.getSoTimeoutMs()); From 629a7adcb61ce75060306fd92120e1081169e1cb Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Wed, 20 Mar 2019 22:02:58 +0000 Subject: [PATCH 10/13] Disable questionalbe test. --- .../modules/fetcher/CookieFetchHTTPIntegrationTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java index 4ea94d22..379b8eec 100644 --- a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java +++ b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java @@ -325,10 +325,11 @@ public class CookieFetchHTTPIntegrationTest extends ProcessorTestBase { * XXX I think browsers differ on this behavior. This is what * org.apache.http.impl.cookie.BrowserCompatSpec does. */ - curi = makeCrawlURI("http://SUBDOMAIN.example.com:7777/"); - fetcher().process(curi); - assertTrue(FetchHTTPTests.httpRequestString(curi).contains("Cookie: foo=bar\r\n")); - assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); + // Disable test as appears to fail under HTTP Client 3.5.7 + //curi = makeCrawlURI("http://SUBDOMAIN.example.com:7777/"); + //fetcher().process(curi); + //assertTrue(FetchHTTPTests.httpRequestString(curi).contains("Cookie: foo=bar\r\n")); + //assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); assertEquals(1, cookieStore.getCookies().size()); } From 044f068d1bbab4f59099ea1f6fedddd4c42b23f0 Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Thu, 21 Mar 2019 00:06:05 +0000 Subject: [PATCH 11/13] Removing outdated test. --- .../fetcher/CookieFetchHTTPIntegrationTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java index 379b8eec..de4b427e 100644 --- a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java +++ b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java @@ -321,16 +321,6 @@ public class CookieFetchHTTPIntegrationTest extends ProcessorTestBase { assertFalse(FetchHTTPTests.httpRequestString(curi).toLowerCase().contains("cookie:")); assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); - /* - * XXX I think browsers differ on this behavior. This is what - * org.apache.http.impl.cookie.BrowserCompatSpec does. - */ - // Disable test as appears to fail under HTTP Client 3.5.7 - //curi = makeCrawlURI("http://SUBDOMAIN.example.com:7777/"); - //fetcher().process(curi); - //assertTrue(FetchHTTPTests.httpRequestString(curi).contains("Cookie: foo=bar\r\n")); - //assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); - assertEquals(1, cookieStore.getCookies().size()); } From dd37598470945ccdab3530ea2d73f8b9091a54e3 Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Thu, 28 Mar 2019 10:14:42 +0900 Subject: [PATCH 12/13] Revert "Upgrade httpclient to 4.5.7 and handle cookies more compliantly" --- .../java/org/archive/modules/fetcher/BdbCookieStore.java | 3 +-- .../org/archive/modules/fetcher/FetchHTTPRequest.java | 2 +- .../modules/fetcher/CookieFetchHTTPIntegrationTest.java | 9 +++++++++ pom.xml | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java b/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java index d900363d..0f047bbc 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java +++ b/modules/src/main/java/org/archive/modules/fetcher/BdbCookieStore.java @@ -20,7 +20,6 @@ package org.archive.modules.fetcher; import java.io.IOException; import java.io.UnsupportedEncodingException; -import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.Iterator; @@ -79,7 +78,7 @@ public class BdbCookieStore extends AbstractCookieStore implements @Override public int size() { return wrapped.size(); } @Override public boolean isEmpty() { throw new RuntimeException("not implemented"); } @Override public boolean contains(Object o) { throw new RuntimeException("not implemented"); } - @Override public Iterator iterator() { return (Iterator) Arrays.asList(wrapped.toArray()).iterator(); } + @Override public Iterator iterator() { throw new RuntimeException("not implemented"); } @Override public Object[] toArray() { return wrapped.toArray(); } @SuppressWarnings("hiding") @Override public T[] toArray(T[] a) { return wrapped.toArray(a); } @Override public boolean add(T e) { throw new RuntimeException("immutable list"); } diff --git a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java index 530eb943..3ee303ee 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java +++ b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java @@ -400,7 +400,7 @@ public class FetchHTTPRequest { if (fetcher.getIgnoreCookies()) { requestConfigBuilder.setCookieSpec(CookieSpecs.IGNORE_COOKIES); } else { - requestConfigBuilder.setCookieSpec(CookieSpecs.DEFAULT); + requestConfigBuilder.setCookieSpec(CookieSpecs.BROWSER_COMPATIBILITY); } requestConfigBuilder.setConnectionRequestTimeout(fetcher.getSoTimeoutMs()); diff --git a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java index de4b427e..4ea94d22 100644 --- a/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java +++ b/modules/src/test/java/org/archive/modules/fetcher/CookieFetchHTTPIntegrationTest.java @@ -321,6 +321,15 @@ public class CookieFetchHTTPIntegrationTest extends ProcessorTestBase { assertFalse(FetchHTTPTests.httpRequestString(curi).toLowerCase().contains("cookie:")); assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); + /* + * XXX I think browsers differ on this behavior. This is what + * org.apache.http.impl.cookie.BrowserCompatSpec does. + */ + curi = makeCrawlURI("http://SUBDOMAIN.example.com:7777/"); + fetcher().process(curi); + assertTrue(FetchHTTPTests.httpRequestString(curi).contains("Cookie: foo=bar\r\n")); + assertFalse(FetchHTTPTests.rawResponseString(curi).toLowerCase().contains("set-cookie:")); + assertEquals(1, cookieStore.getCookies().size()); } diff --git a/pom.xml b/pom.xml index 7df433e1..033cd488 100644 --- a/pom.xml +++ b/pom.xml @@ -127,12 +127,12 @@ http://maven.apache.org/guides/mini/guide-m1-m2.html org.apache.httpcomponents httpclient - 4.5.7 + 4.3.6 org.apache.httpcomponents httpmime - 4.5.7 + 4.3.6 From 2c6ab5a44a7b7393d541a8f6ef56f7c5146aca45 Mon Sep 17 00:00:00 2001 From: Noah Levitt Date: Thu, 4 Apr 2019 17:06:01 -0700 Subject: [PATCH 13/13] replace System.err.println with logger.info --- .../java/org/archive/crawler/frontier/WorkQueueFrontier.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java b/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java index d08cd566..4d651ece 100644 --- a/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java +++ b/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java @@ -442,8 +442,8 @@ implements Closeable, synchronized(wq) { wq.noteDeactivated(); inProcessQueues.remove(wq); - if(wq.getCount()==0) { - System.err.println("deactivate empty queue?"); + if (wq.getCount() == 0) { + logger.info("deactivate empty queue? " + wq.getClassKey()); } synchronized (getInactiveQueuesByPrecedence()) {