Closed Bug 1270681 Opened 4 years ago Closed 4 years ago

Add mochitest to ensure that the HTTP Cache is respecting usercontextId

Categories

(Core :: DOM: Security, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Iteration:
49.3 - Jun 6
Tracking Status
firefox50 --- fixed

People

(Reporter: tanvi, Assigned: jhao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-active][OA-testing])

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #962374 +++
Whiteboard: [necko-backlog][OA][userContextId] → [domsecurity-backlog][OA][userContextId]
Why is this marked as an [OA] bug? I can see how it is important for containers (though how secure/
anonymous are we going for in containers?) but not sure how it relates to origin refactoring effort. Given how long that bug has been around, I don't think this is realistic to make 49.
Flags: needinfo?(tanvi)
Flags: needinfo?(tanvi)
Priority: -- → P1
Assignee: nobody → huseby
Whiteboard: [domsecurity-backlog][OA][userContextId] → [domsecurity-backlog][OA][userContextId][tor]
Whiteboard: [domsecurity-backlog][OA][userContextId][tor] → [domsecurity-backlog][OA][userContextId]
Dave did you get a chance to look into this bug in more detail. I put in a date based on the 2 week guess, but let me know if this turns out to be more complex than expected.
Iteration: --- → 49.3 - Jun 6
Flags: needinfo?(huseby)
Another thought from a 1270680: Does this patch handle upgrade and downgrade of profiles? For example, if we land this in nightly, and then someone opens the profile in nightly - will the existing cache be lost or cause issues somehow? Also vice versa when they take the profile back to release version and the cache now contains extra values. Probably not the end of the world if these cache entries are purged, but we'd still need to handle it, right? Or does the patch handle this already?
Reassigning for now since Dave is out this week and has a couple other bugs he's working on.
Assignee: huseby → nobody
Flags: needinfo?(huseby)
The HTTP cache was already made origin attribute aware in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1165269.  So converting this to a testing bug.

There are already some tests that landed in bug 1165269 that include appid and inbrowser.  We may be able to just expand on those existing tests.
Priority: P1 → P2
Summary: Content Cache should respect OriginAttributes → Add mochitest to ensure that the HTTP Cache is respecting usercontextId
Whiteboard: [domsecurity-backlog][OA][userContextId] → [domsecurity-backlog][OA-testing]
(In reply to Tanvi Vyas - out until 6/27 [:tanvi] from comment #6)
> The HTTP cache was already made origin attribute aware in bug
> https://bugzilla.mozilla.org/show_bug.cgi?id=1165269.  So converting this to
> a testing bug.
> 
> There are already some tests that landed in bug 1165269 that include appid
> and inbrowser.  We may be able to just expand on those existing tests.

I guess Tanvi meant http://searchfox.org/mozilla-central/source/netwerk/test/unit/test_cache_jar.js
I'll add userContextId to this test.
Assignee: nobody → jhao
Status: NEW → ASSIGNED
Hi, Tanvi. Is this what you expected of this bug?
Attachment #8764840 - Flags: feedback?(tanvi)
Comment on attachment 8764840 [details] [diff] [review]
Add a test to ensure that the HTTP Cache is respecting usercontextId.

(In reply to Jonathan Hao [:jhao] from comment #8)
> Created attachment 8764840 [details] [diff] [review]
> Add a test to ensure that the HTTP Cache is respecting usercontextId.
> 
> Hi, Tanvi. Is this what you expected of this bug?

Yes.  Please ask for a necko peer review.  Thanks Jonathan!
Attachment #8764840 - Flags: feedback?(tanvi) → feedback+
Comment on attachment 8764840 [details] [diff] [review]
Add a test to ensure that the HTTP Cache is respecting usercontextId.

Honza, would you review this patch? Thanks.
Attachment #8764840 - Flags: review?(honzab.moz)
Whiteboard: [domsecurity-backlog][OA-testing] → [domsecurity-active][OA-testing]
Comment on attachment 8764840 [details] [diff] [review]
Add a test to ensure that the HTTP Cache is respecting usercontextId.

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

::: netwerk/test/unit/test_cache_jar.js
@@ +44,5 @@
>    };
>    return chan;
>  }
>  
> +// [userContextId, appId, inIsolatedMozBrowser, expected_handlers_called]

better would be to add your new arg as the last one, you would save the shift everywhere.  I won't do any double checks on it.
Attachment #8764840 - Flags: review?(honzab.moz) → review+
Move the new argument to the last as Honza suggested.
Attachment #8771915 - Flags: review+
Attachment #8764840 - Attachment is obsolete: true
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb72efabe34e
Add a test to ensure that the HTTP Cache is respecting usercontextId. r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cb72efabe34e
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.