1270681 - Add mochitest to ensure that the HTTP Cache is respecting usercontextId

Status: RESOLVED FIXED

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

Description

[Tanvi Vyas[:tanvi]]

+++ This bug was initially created as a clone of Bug #962374 +++

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

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.

Comment 2

[Paul Theriault [:pauljt] (no longer reading bugmail)]

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.

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Especially in respect to this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=962374#c2

Comment 4

[Paul Theriault [:pauljt] (no longer reading bugmail)]

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?

Comment 5

[Tanvi Vyas[:tanvi]]

Reassigning for now since Dave is out this week and has a couple other bugs he's working on.

Comment 6

[Tanvi Vyas[:tanvi]]

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.

Comment 7

[Jonathan Hao (inactive) [:jhao]]

(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.

Comment 8

[Jonathan Hao (inactive) [:jhao]]

Attachment: Add a test to ensure that the HTTP Cache is respecting usercontextId.

Hi, Tanvi. Is this what you expected of this bug?

Comment 9

[Tanvi Vyas[: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!

Comment 10

[Jonathan Hao (inactive) [:jhao]]

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.

Comment 11

[Honza Bambas (:mayhemer)]

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.

Comment 12

[Jonathan Hao (inactive) [:jhao]]

Attachment: Add a test to ensure that the HTTP Cache is respecting usercontextId.

Move the new argument to the last as Honza suggested.

Comment 13

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3e91a732098&selectedJob=24095848

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb72efabe34e
Add a test to ensure that the HTTP Cache is respecting usercontextId. r=mayhemer

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cb72efabe34e