iipc / openwayback

The OpenWayback Development
http://www.netpreserve.org/openwayback
Apache License 2.0
486 stars 275 forks source link

Support brotli content-encoding #395

Closed ato closed 5 years ago

ato commented 5 years ago

Adds support for Content-Encoding: br which is now broadly supported by browsers and turning up in WARCs created by tools like Webrecorder.

ldko commented 5 years ago

Hey @ato, thanks for submitting this! I looked over it somewhat quickly, and I think it looks good, but I can look at it more later. Initial thoughts: perhaps update the Javadoc comment for decodeResource in TextReplayRenderer.java, update the release notes, and maybe add a test in DecodingResourceTest to hit the default case in the switch statement.

ato commented 5 years ago

Thanks for the review @ldko. Updated to address your comments.

Adding the tests made me realise the existing fallback behaviour when Content-Encoding: gzip is present but the content is not actually gzipped was losing the first 2 bytes of the stream. So I fixed that too. I'm not sure that fallback behaviour is worthwhile (I wonder if browsers actually handle false content-encoding headers or not) but since it already exists it should at least not lose data.

Sorry for the force-push. I messed up resolving the release-notes merge conflict (Windows line endings gah!) and so reset and rebased to clean up.