Skip to content

Conversation

liady
Copy link
Collaborator

@liady liady commented Aug 13, 2025

See #76 - exposed raw HTML as srcDoc might be a security risk when used with other 3rd party scripts on the page.
This PR changes the default from a raw HTML srcDoc to a Blob URL src.
In embeding environments where CSP block Blob URLs - the useSrcDoc prop can be set to revert this behavior (and use srcDoc as before).

@liady liady marked this pull request as ready for review August 13, 2025 19:11
Copy link

cloudflare-workers-and-pages bot commented Aug 13, 2025

Deploying mcp-ui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 37b588d
Status: ✅  Deploy successful!
Preview URL: https://caee3775.mcp-ui.pages.dev
Branch Preview URL: https://feat-render-blob-instead-src.mcp-ui.pages.dev

View logs

Copy link
Owner

@idosal idosal left a comment

Choose a reason for hiding this comment

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

Awesome! We may want to set a clearer name for the prop. Note that this is technically a breaking change

Copy link

@weizman weizman left a comment

Choose a reason for hiding this comment

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

Looks good (focused solely on blob logic part)

},
);
}
if (iframeSrcBlobUrl.current && typeof window?.URL?.revokeObjectURL === 'function') {
Copy link

Choose a reason for hiding this comment

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

If you managed to create the blob since the create function existed, it necessarily translates to the revoke function existing as well, therefore check might be redundant (nit)

@aharvard
Copy link
Contributor

Cool idea here.

Is there a limit within any given browser to how many characters a blob URL can be for an iframe src?

I imagine it's possible for rawHTML to be huge if massive script bundles and large style blocks get inlined. Hopefully, this won't be the norm, but I am curious about the upper bound edge case (if one exists).

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.

5 participants