F5 and other normal ways to reload the page don't work with local .json file opened in JSON viewer

VERIFIED FIXED in Firefox 54

Status

defect
P3
normal
VERIFIED FIXED
2 years ago
4 months ago

People

(Reporter: 684sigma, Assigned: ckerschb)

Tracking

({regression})

53 Branch
Firefox 55
Dependency tree / graph

Firefox Tracking Flags

(firefox52 disabled, firefox-esr52 disabled, firefox53+ wontfix, firefox54+ verified, firefox55+)

Details

Attachments

(1 attachment)

I found a bug in Firefox Beta 53. It also happens in Nightly 55. Doesn't happen in Beta 52, because it's a new feature in Beta 53. But it worked well, before I updated from Beta 53b5 to Beta 53b8. Therefore I'm adding keyword "regression".
The bug is: Pressing Ctrl+R / F5, clicking reload button, or "Reload" menuitem doesn't reload local .json file opened in JSON viewer. I noticed this bug when I changed something in a local .json file and tried to reload it in browser to check the result.
Here's how to reproduce it:

1. Download attachment 8853723 [details] and open it in a new tab
2. Press F5

Result: F5 (and other mentioned things) has no effect
Expected: F5 works perfectly
Has STR: --- → yes
Keywords: regression
> Security Error: Content at moz-nullprincipal:{44f6f526-8a01-b248-b2f3-9e18ed863ace} may not load or link to file:///.../blob.json

Christoph, is there a sane way around this problem?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(ckerschb)
(In reply to :Gijs from comment #2)
> > Security Error: Content at moz-nullprincipal:{44f6f526-8a01-b248-b2f3-9e18ed863ace} may not load or link to file:///.../blob.json
> 
> Christoph, is there a sane way around this problem?

Gijs, I took a closer a look at this problem, so let me summarize: The initial load of the JSON file uses a triggeringprincipal of Systemprincipal but then gets overruled by [1] which forces all the principals in the loadInfo to be reset to a freshly created  NullPrincipal. In turn this resetting of principals causes the history entry to be created using a NullPrincipal as the triggeringPrincipal which then obviously is not allowed to load a file:// URI.

The fact that we have a principalToInherit which in this case would and should be a NullPrincipal makes me wonder if resetting the triggeringPrincipal here [2] is even correct. Probably we should *not* reset the triggeringPrincipal. In my opinion it should remain the same. In the case here it would remain the SystemPrincipal which actually triggered the initial load of the JSON which would also fix the problem described in this bug.

With that changed the test landed within Bug 1333210 still works and we would have to update test_forceinheritprincipal_overrule_owner.html to reflect those changes.


[1] https://hg.mozilla.org/releases/mozilla-aurora/rev/8d73c486834d#l1.15
[2] https://hg.mozilla.org/mozilla-central/rev/3b7254b9fe5f#l2.71
Flags: needinfo?(ckerschb)
Makes sense, though all this stuff is nettly. Boris, does comment #3 make sense to you, too?
Flags: needinfo?(bzbarsky)
There's another very similar bug: Browser shows empty page when restoring tab with local .json file
1. Download attachment 8853723 [details] and open it in a new tab
2. Close tab, restore tab

I haven't filed it yet, but probably connected. I see the same error in browser console:
Security Error: Content at moz-nullprincipal:{964d6986-b3b0-4a0f-ba4a-e23fe748e785} may not load or link to file:///.../example%20of%20JSON%20file,%20by%20reporter.json.
Yes, I don't see why we would reset anything _other_ than the principalToInherit....
Flags: needinfo?(bzbarsky)
Assignee: nobody → ckerschb
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
I fixed that up, and I think we should uplift that change.

I verified that:
*) STRs from comment 0 work again
*) test/browser_jsonview_valid_json.js  works correctly
*) test_forceinheritprincipal_overrule_owner.html works correctly
*) STRs from comment 5 work again (but someone should verify once again)
Attachment #8854093 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8854093 [details] [diff] [review]
bug_1352778_reset_triggeringPrincipal.patch

Review of attachment 8854093 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #8854093 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f8f3c6fecd9
Do not reset triggeringPrincipal but only principalToInherit within loadInfo when forced to. r=gijs
https://hg.mozilla.org/mozilla-central/rev/1f8f3c6fecd9
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Are you sure it is unaffected for 52? If it's caused by bug 1333210 that landed for 52.
I forget regularly to set flags for ESR, and I don't have access to bug 1333210, but ESR is probably affected too. :)

NB/ someone can add bug 1333210 to the "blocks" field, please.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #11)
> Are you sure it is unaffected for 52? If it's caused by bug 1333210 that
> landed for 52.

The JSON viewer is disabled by default on 52.
Gijs, can I convince you to fill out the uplift request?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8854093 [details] [diff] [review]
bug_1352778_reset_triggeringPrincipal.patch

Approval Request Comment
[Feature/Bug causing the regression]: security issue with the JSON viewer
[User impact if declined]: no obvious way to reload JSON files once they show in the viewer, potentially other, similar issues with the viewer
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: see comment #0
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: uh, well, it's the last week of beta, so inasmuch as changes are normally non-risky in the last week of beta...
[Why is the change risky/not risky?]: We're renaming an XPCOM API and changing what it does. The API was new in 52, and no add-ons use it, so the API change should be safe externally. For the actual effect of the change, it should be limited to the JSON viewer because in-tree, it is currently the only (non-test) consumer of this API. We have tests verifying the API works as advertised, and we have separate tests that the effect on the json viewer is as it should be, so given that those pass, I think that's all the areas I can think of for regressions covered.
[String changes made/needed]: no
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8854093 - Flags: approval-mozilla-beta?
Attachment #8854093 - Flags: approval-mozilla-aurora?
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #14)
> Gijs, can I convince you to fill out the uplift request?

Done. Can you file a followup bug to create an automated regression test for comment #0 and potentially comment #5? Thanks!
Flags: needinfo?(ckerschb)
(In reply to :Gijs from comment #16)
> Can you file a followup bug to create an automated regression test for
> comment #0 and potentially comment #5? Thanks!

I filed Bug 1353736 to get that done.
Depends on: 1353736
Flags: needinfo?(ckerschb)
Too late for a fix for 53, minor carryover regression, no one assigned, so these don't seem like a high priority.
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
Attachment #8854093 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Verified fixed using the latest Nightly 55.0a1 (2017-04-07) on Windows 10, Ubuntu 16.04 and Mac OS X 10.10.
Flags: needinfo?(brindusa.tot)
Comment on attachment 8854093 [details] [diff] [review]
bug_1352778_reset_triggeringPrincipal.patch

Fix a regression related to reload JSON files in the viewer and was verified. Aurora54+.
Attachment #8854093 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I’ve reproduced the issue described in comment 0 using Firefox  55.0a1 (Build Id:20170401030204) and verified that the issue is not reproducible anymore using Firefox 54.0b1 (Build Id: 20170420011827) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.