fix(webview): drop allowFileAccessFromFileURLs in reader WebView#1277
Merged
Conversation
…Access in WebViewLayout
WebViewLayout.get() toggled two settings inside the ReadingFontsPreference
when block: allowFileAccess and allowFileAccessFromFileURLs, both set to
true for the GoogleSans and External preferences.
allowFileAccessFromFileURLs lets scripts running on a file:// page issue
XHR requests against other file:// resources. None of the article HTML
fed to this WebView is a file:// document (it is delivered via
loadDataWithBaseURL or loadUrl on http content), so the flag has no
positive effect for either font mode, and it broadens what a malicious
sub-resource could read if the WebView ever ended up on a file URL.
allowFileAccess for the GoogleSans branch is also unnecessary. The
Google Sans assets are bundled and reached via file:///android_asset/,
which works even when allowFileAccess is left at its default. The
External branch genuinely loads a user-picked font from device storage,
so the flag stays in place there.
Net effect:
GoogleSans: no behaviour change, two unsafe flags removed
External: one unsafe flag removed, file access still permitted
for the user-chosen font file
Member
|
Sweet, merging this now 🙌 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1276.
WebViewLayout.get()enables twoWebSettingsflags for theGoogleSansandExternalreading-fonts preferences:Why this PR removes some of them:
allowFileAccessFromFileURLs = trueonly takes effect on a page whose URL has afile://scheme, and even then it just lets scripts on that page XHR otherfile://resources. The reader WebView is fed vialoadDataWithBaseURLand HTTP URLs, not a file URL, so the flag does not change how Google Sans or the user-picked external font is loaded. It is removed in both branches.allowFileAccess = trueis not needed for theGoogleSansbranch either. Bundled fonts underfile:///android_asset/load even whensetAllowFileAccess(false)is in force on every supported Android version, which is the reason Android docs treatandroid_assetas a special case. The flag is removed there.Externalbranch genuinely loads a font file the user picked from device storage, soallowFileAccess = trueis preserved in that branch.On
minSdk <= 29(this project shipsminSdk = 26)allowFileAccessandallowFileAccessFromFileURLsdefault totrue, so the removal inGoogleSansis effectively a tightening of WebView posture rather than a redundant change on older devices. On API 30+ both flags default tofalseand the change is a no-op for those devices.