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

Should iframe have setSrcdoc method? #117

Open
securityMB opened this issue Jul 26, 2021 · 11 comments
Open

Should iframe have setSrcdoc method? #117

securityMB opened this issue Jul 26, 2021 · 11 comments
Milestone

Comments

@securityMB
Copy link

The current version of the spec extends Element with setHtml method. Another use-case for sanitization (although less common) is to set srcdoc of iframes, for instance:

iframe.srcdoc = sanitize(dirty);

It seems that currently there's no canonical way of doing so in the sanitizer API.

Maybe we should extend HTMLIFrameElement with setSrcdoc method (similarly to setHtml), which would keep in mind that srcdoc is normally being parsed as the document (not as a fragment)?

@Sora2455
Copy link

It took me a moment to realise what the point of this was, because I forgot that people used srcdoc without sandbox.

@mozfreddyb
Copy link
Collaborator

We've talked about this in our regular check-in and I had a thought that didn't totally convince @otherdaniel:
Maybe iframe.setHTML could do exactly this? Currently it throws for iframes, because they don't contain content. Here's a proposal:

setHTML()
if the current element is an iframe element:

  • parse the supplied input with a Document parser
  • sanitize that document in-place using the given sanitizer (or the default)
  • serialize the document to string (yuck!) and assign it to the element's srcdoc attribute

@annevk
Copy link
Collaborator

annevk commented Oct 18, 2023

This makes some sense to me, but should probably be proposed against whatwg/html instead. @securityMB mind proposing it there?

(It does need some thought as to what the actual string ends up being as we don't really want to expose serialization per se. Perhaps the srcdoc attribute doesn't end up getting set, but it's something internal instead.)

@annevk
Copy link
Collaborator

annevk commented Apr 3, 2024

I suggest we close this and if there continues to be demand that will hopefully result in someone feeling motivated enough to request it in the proper place (whatwg/html).

@lukewarlow
Copy link
Contributor

My two cents here are that this should be part of setHTML rather than a new function, and maybe even require an option on the second param. But agree with Anne that for now it's probably fine not to cover?The trusted types API will exist as a fallback security measure for any sinks such as this that aren't covered.

Also would the getHTML spec help with the serialization side of things?

@annevk
Copy link
Collaborator

annevk commented Apr 17, 2024

I guess you are combining this with #124 somehow? I'm not really sure how this fits into setHTML(). Because <iframe>.setHTML() shouldn't mutate its srcdoc attribute. And there should probably be a way to safely set its srcdoc attribute without having to create a new iframe element.

@lukewarlow
Copy link
Contributor

I was referencing a comment above.

#117 (comment) (though I realise it might be outdated at this point)

@mozfreddyb
Copy link
Collaborator

Document.parseHTML returns a new document. Could there be a document.setHTML such that one could use iframe.contentDocument.setHTML?

@mozfreddyb mozfreddyb added the v1 label Apr 17, 2024
@mozfreddyb mozfreddyb added this to the v1 milestone Apr 17, 2024
@mozfreddyb mozfreddyb removed the v1 label Apr 17, 2024
@annevk
Copy link
Collaborator

annevk commented Apr 17, 2024

That's an interesting idea, but that would not work for this case:

const frame = document.createElement("iframe");
// frame.contentDocument is still null at this point

@Sora2455
Copy link

How about:

const frame = document.createElement("iframe");
frame.src = "about:blank";
document.body.appendChild(frame);
// contentDocument now exists (tested in Firefox and Edge)
frame.contentDocument 

@annevk
Copy link
Collaborator

annevk commented Apr 18, 2024

Well sure, but that doesn't seem like what you'd want?

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

No branches or pull requests

5 participants