HtmlExtractor: allow relative hrefs in the base element

[anjackson]

I believe this ensures Heritrix3 deals with relative base hrefs. (fixes #208)

[ato]

Looks good to me. I think JerichoExtractorHtml has the same problem.

[anjackson]

Hm, well, this now breaks other tests. Elsewhere, BAD BASE tests are in play that assume base href should be absolute.

Not sure what to do about this.

[ato]

Hm. So I think the problem is that HTML4 specified that base URLs must be absolute. Presumably browsers at the time ignored absolute URLs hence the BADBASE tests. However HTML5 allows them to be relative but only the first is obeyed and any subsequent base element is ignored.

So I guess the options for Heritrix's behaviour are:

1. Follow what contemporary browsers do: allow relative URLs and ignore base tags other than the first.
2. Generate candidates for every possible case.
3. Change behaviour depending on the doctype.

Personally I would go with option 1 since it's simple and matches what browsers do. I would consider option 2 speculative link extraction.

[anjackson]

I'm happy to implement option 1. Option 2 gets quite messy as it would mean storing all potential base URIs and generating all potential URL combinations (i.e. for L links and B base URIs we'd need to emit L x B links!) Option 3 is doable too I guess, with a fallback to option 1 when the DOCTYPE is not known.

I'm not really sure what to do with the tests, as they appear to be testing what happens when you get bad URIs, and happen to use the HTML4 base URI convention to make it happen. Having taken that away, I am struggling to make a test case that actually does create a bad URI!

[ato]

I'm not really sure what to do with the tests, as they appear to be testing what happens when you get bad URIs

If we're going with option 1 I think we should replace those bad url tests entirely. They're just plain wrong for modern html. We should instead test relative base hrefs and a page with several base tags.

[anjackson]

The problem is that the failing test is called BadURIsStopPageParsingSelfTest i.e. it's about hitting bad URIs (that it uses base hrefs to do it seems to be incidental). Having modified the base href handling, I don't know how to make the BadURIs test go off. I guess we could use very, very long URIs? If I add 'bad' characters it just escapes them.

[ato]

Looks like this is the original issue: https://webarchive.jira.com/browse/HER-25

I don't know how to make the BadURIs test go off.

I'm confused, why would we want them to? Isn't the point of the original issue that Heritrix should NOT choke on bad URIs? (ie the name of the test is describing the problem addressed not the desired behaviour)

[ato]

Those tests look all wrong to me. I'm pretty sure modern browsers would apply the base urls in all cases (one.html two.html html). If there's extra slashes or illegal characters ir whatever modern browsers just normalise and escape them. The schemaless url is treated as a relative URL. So I still think the tests are wrong and should be removed. (Or inverted so they enforce that goodone.html etc are actually not found)

[anjackson]

Yes, and the test appears to be designed to check that the page parsing does not stop if the extractor hits a bad URI. If I can't create a URI error, then I can't make the test test anything.

[anjackson]

Okay, so from a Real Life Log File^(TM) I find three types of error:

• URI length > 2083
• URI too short to be a meaningful URI
• Contains non-LDH characters due to a %20 in the host name.

I think the latter is probably easiest to use.

[ato]

If I can't create a URI error, then I can't make the test test anything.

Ah. How about this?

UURIFactory.getInstance("http://a:b/");
-->
org.apache.commons.httpclient.URIException: invalid port number
	at org.apache.commons.httpclient.URI.parseAuthority(URI.java:2248)
	at org.archive.url.LaxURI.parseAuthority(LaxURI.java:190)
	at org.archive.url.LaxURI.parseUriReference(LaxURI.java:359)
	at org.apache.commons.httpclient.URI.<init>(URI.java:147)

[anjackson]

Right, so, finally got the tests working. One of the other tests assumed BaseURI would be set every time a base element was read, rather than just the first, so I had to modify that too.

[anjackson]

This was the change needed because BaseURI can now only be set once in ExtractorHTML

[kris-sigur]

@anjackson It looks fine, but unfortunately you caught me minutes before I'm out the door for a holiday so I can't properly vet it until the end of the month.

[anjackson]

Hey @nlevitt @kris-sigur or anyone able to take a quick look at this?

[nlevitt]

Looks fine to me.