Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify data URLs are not publication resources #2494

Merged
merged 10 commits into from
Dec 6, 2022
Merged

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Nov 29, 2022

This one got me pretty twisted up trying to solve.

There's a second case I thought of where you could technically use a data URL in the manifest: fallback resources. You could encode a CMT image as a data URL for an img element fallback, for example. But do we really want to support this or having a nav doc not in the spine as a data URL? I'm guessing not.

Assuming that's the case, what I've done is try to make clearer that data URLs are not unique from their container resource. They must not be listed in the manifest, but because they have media types separate from their container they are still subject to fallback restrictions.

I've also assumed that we don't care what people do with linked resources, so data URLs are allowed in link elements in the metadata.

Anyway, take a good look at this PR as it's a bit of a convoluted problem.

Fixes #2485


Preview | Diff

Copy link
Member

@rdeltour rdeltour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK to me, pending some (minor) edit suggestions.

Also, if we allow data URLs on linked resources, we may want to change the following (from the link element definition):

The media-type attribute is OPTIONAL when a linked resource is located outside the EPUB container, as more than one media type could be served from the same URL [url]. EPUB creators MUST specify the attribute for all linked resources within the EPUB container

to

The media-type attribute is OPTIONAL when a linked resource is located outside the EPUB container (as more than one media type could be served from the same URL [url]), or when the resource is encoded as a data URL (in the href attribute). EPUB creators MUST otherwise specify the attribute for all linked resources located within the EPUB container.

epub33/core/index.html Outdated Show resolved Hide resolved
epub33/core/index.html Outdated Show resolved Hide resolved
epub33/core/index.html Outdated Show resolved Hide resolved
epub33/core/index.html Outdated Show resolved Hide resolved
@mattgarrish
Copy link
Member Author

we may want to change the following (from the link element definition):

Ya, that's fine with me. I just reworked it into a bullet list as there's a lot going on in the sentence.

Since linked resources are kind of a niche anyway, allowing data URLs for them doesn't bother me. But if we don't want them at all in the package document I can go that way, too.

epub33/core/index.html Outdated Show resolved Hide resolved
@mattgarrish
Copy link
Member Author

Should we ban data URLs in link elements and be done with them in the package document? Given the problematic security of data URLs, and the limited uses of links, it seems like a prudent thing to do.

@rdeltour
Copy link
Member

rdeltour commented Dec 2, 2022

Should we ban data URLs in link elements and be done with them in the package document?

Yeah, no strong opinion here, but I'm pretty confident no one will complain if we ban them.

@iherman
Copy link
Member

iherman commented Dec 3, 2022

Should we ban data URLs in link elements and be done with them in the package document? Given the problematic security of data URLs, and the limited uses of links, it seems like a prudent thing to do.

Same as @rdeltour.

@mattgarrish
Copy link
Member Author

I've updated the PR to disallow data urls in the package document, but it's meant some rewriting of the data URLs section.

It didn't make sense to lead into the restriction in the package document by talking about publication resources, so I added the restriction to the list of scenarios that can result in insecure rendering. Instead of saying these will result in top-level content documents/browsing context, I shifted it to "can result" to justify having the link element in there. It's a bit of an outlier, as we don't know how a reading system will present a link.

I then shifted the text about data URLs not being unique publication resources down to the paragraph about their still having their own media types so being subject to fallback requirements.

Otherwise, I just undid the bits that were dealing with the link element - the one note about allowing data urls and the paragraph about linked resources not needing a media-type attribute when expressed as data urls.

@mattgarrish
Copy link
Member Author

As this has been open for a week now, I'm going to merge.

@mattgarrish mattgarrish merged commit 0667c09 into main Dec 6, 2022
@mattgarrish mattgarrish deleted the editorial/issue-2485 branch December 6, 2022 12:48
rdeltour added a commit to w3c/epubcheck that referenced this pull request Dec 15, 2022
EPUB 3.3 now disallows data URLs everywhere in the package document
(in both `item` and `link` elements).

See w3c/epub-specs#2494

Fix #1446
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.

some confusion around data URLs
3 participants