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

fix: crash loading non-standard schemes in iframes #35485

Merged
merged 1 commit into from Aug 31, 2022

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Aug 29, 2022

Description of Change

Closes #35462.
Closes #28407.

This fixes a crash that occurs when loading non-standard schemes from iframes. This was happening because
ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin contains explicit exceptions to allow built-in non-standard schemes, but does not check for non-standard schemes registered by the embedder.

Upstream, CRBUG:1081397 contains several paths forward - here I chose to swap out theCHECK in navigation_request.cc from policy->CanAccessDataForOrigin topolicy->CanCommitOriginAndUrl.

CL opened at CL:3856266

Checklist

Release Notes

Notes: Fixed a crash that occured when loading non-standard schemes from iframes.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/19-x-y target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 29, 2022
@codebytere codebytere requested review from a team as code owners August 29, 2022 10:18
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 29, 2022
@codebytere codebytere force-pushed the fix-iframe-non-standard-scheme-crash branch from 71707fa to 17c4c86 Compare August 29, 2022 12:54
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 30, 2022
@codebytere codebytere force-pushed the fix-iframe-non-standard-scheme-crash branch from 17c4c86 to 299c293 Compare August 30, 2022 16:25
@codebytere codebytere requested a review from ckerr August 30, 2022 16:34
@codebytere codebytere merged commit e0fb5cb into main Aug 31, 2022
@codebytere codebytere deleted the fix-iframe-non-standard-scheme-crash branch August 31, 2022 08:08
@release-clerk
Copy link

release-clerk bot commented Aug 31, 2022

Release Notes Persisted

Fixed a crash that occured when loading non-standard schemes from iframes.

@trop
Copy link
Contributor

trop bot commented Aug 31, 2022

I have automatically backported this PR to "19-x-y", please check out #35515

@trop
Copy link
Contributor

trop bot commented Aug 31, 2022

I have automatically backported this PR to "20-x-y", please check out #35516

@trop
Copy link
Contributor

trop bot commented Aug 31, 2022

I have automatically backported this PR to "21-x-y", please check out #35517

@trop trop bot removed target/20-x-y target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 31, 2022
@george-thomas-hill
Copy link

Thank you for doing this!

@trop trop bot added merged/21-x-y PR was merged to the "21-x-y" branch. and removed in-flight/21-x-y labels Sep 21, 2022
@george-thomas-hill
Copy link

@codebytere :

Update:

I'm still finding (in Electron 21.2.0) that loading a PDF in an iFrame fails to display the PDF, and loading a PDF in a webview crashes Electron.

Here's an Electron Fiddle gist that demonstrates this:

https://gist.github.com/george-thomas-hill/c3179e6eddeb1a2715201a20dbf95015

Could you please take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/21-x-y PR was merged to the "21-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
4 participants