Skip to content

Add a nil guard to prevent crashes when a link's media type cannot be parsed - #324

Open
leonardr wants to merge 5 commits into
readium:developfrom
bookshop-org:bookshop/v0.15.1
Open

Add a nil guard to prevent crashes when a link's media type cannot be parsed#324
leonardr wants to merge 5 commits into
readium:developfrom
bookshop-org:bookshop/v0.15.1

Conversation

@leonardr

Copy link
Copy Markdown

This branch fixes a problem I've seen in real EPUBs (sample available on request) which causes a crash on load.

The problem is triggered by a link in the OPF with a media type that causes mediatype.New to error; in my case it was "application/application/x-font-ttf". The Link object ends up with a nil MediaType. There are a number of places in the code that call functions on link.MediaType without a nil guard, which causes the crash..

In this branch I added nil guards everywhere in the code I think is relevant. The end result should be that a link with a nil media type is not anything in particular: not a bitmap, not HTML, etc.

An alternative implementation would be to define a sentinel "unknown" MediaType to stand in for the case where mediatype.New can't parse the actual media type. Then we can guarantee mediatype.New always returns a MediaType and the nil guards won't be necessary. That's a more systematic change, so before working on it, I want to understand your general policy on having go-toolkit handle invalid data in EPUBs.

@chocolatkey

Copy link
Copy Markdown
Member

@leonardr sorry for the delay. Your idea to add nil guards is a good one. I wouldn't necessarily be against adding an unknown mediatype as you suggested as well.

I want to understand your general policy on having go-toolkit handle invalid data in EPUBs.

In general, the policy here is to parse a very wide range of EPUBs, and we know that producers of EPUBs don't always do things right. Since the go-toolkit needs to deal with EPUBs out in the while at large scale (such as with your company or mine), it should be designed to be tolerant to poorly authored EPUBs. That's also why I'll be adding more and more heuristics to the toolkit for things such as detecting comic (manga, webtoon) EPUBs and converting to Divina based on special metadata properties etc. because we know the reality is that many EPUBs to not pass EPUBCheck or adhere to W3C recommendations.

real EPUBs (sample available on request)

Please do, I can modify and redact the EPUB and we can make a more thorough Go test for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants