1264572 - Test the Isolate the Image Cache per url bar domain (Tor 13749.2)

Status: RESOLVED FIXED

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

Description

[Dave Huseby [:huseby]]

Attachment: the Tor Browser patch (WIP)

Tor patch for testing isolation of the image cache.

Comment 1

[Arthur Edelstein [:arthur]]

Here's a tracking link to the tor patch:
https://torpat.ch/13749
The "cache" test includes testing for images.

Comment 2

[Dave Huseby [:huseby]]

try: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66fcb5b647cb

Comment 3

[Dave Huseby [:huseby]]

Attachment: Bug_1264572.patch

This patch adds a setup function to the testing framework so that local state can be modified before the tabs are created.

This test also confirms that the image cache isolation is working for first party domains.

Comment 4

[Tim Huang[:timhuang]]

Comment on attachment 8790986 [details] [diff] [review]
Bug_1264572.patch

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

There are some problems need to be fixed here. Also add baku as a reviewer.

::: browser/components/originattributes/test/browser/browser_imageCacheIsolation.js
@@ +13,5 @@
> +let server = new HttpServer();
> +server.registerPathHandler('/image.png', imageHandler);
> +server.registerPathHandler('/file.html', fileHandler);
> +server.start(-1);
> +

The Http server should be stopped after the test is finished. Something like:

```js
registerCleanupFunction(() => {
  server.stop(() => {
    server = null;
  });
});
```

@@ +35,5 @@
> +  response.bodyOutputStream.write(body, body.length);
> +}
> +
> +function doSetup() {
> +  gHits = 0;

I think the image cache should be cleared here as well as you reset the gHits. Because the remaining cache would affect testing results.

::: browser/components/originattributes/test/browser/head.js
@@ +299,5 @@
>      this._add_task(function* (aMode) {
> +      if (aSetupFunc) {
> +        yield aSetupFunc();
> +      }
> +

The setup function should not be called here, it should be called right before tabs are created. The framework will test two cases of a given testing mode, including isolated tabs and non-isolated tabs. For example, it will test two tabs with the same first party domain and then different first party domains in the testing of first party isolation. So, if you put setup function here, only the first case will have a clean state, but the second one will not.

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8790986 [details] [diff] [review]
Bug_1264572.patch

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

Tim's comments are correct.

Comment 6

[Dave Huseby [:huseby]]

Attachment: WIP with clean up and image cache clearing code added

This is a WIP updated patch that includes clean up functions and image cache clearing.  The image cache clearing doesn't seem to work or that the image cache first party isolation isn't working.  Something is broken.  I'm trying to debug it.

Comment 7

[Dave Huseby [:huseby]]

Do either of you have any idea why the image cache clearing doesn't seem to work?  The tests that fail are the second run of the non-isolated test where both image loads come from the cache when the first one should come from the web server.

Comment 8

[Tim Huang[:timhuang]]

I have tested your test case with enabling logging of imgLib. It shows that the image cache clearing does work, but it seems the network request never comes out even if the image cache miss at the second run. I think the reason could be that the image has been cached by the Http cache, so there will be no real network request even if the image cache miss.

Comment 9

[Dave Huseby [:huseby]]

Tim, how did you enable the imgLib logging?

Comment 10

[Dave Huseby [:huseby]]

Attachment: Bug_1264572.patch r+ :baku

r+ :baku

This updated patch adds image cache and network cache clearing between tests so we can be certain that the origin attribute isolation is working correctly.

Comment 11

[Dave Huseby [:huseby]]

try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b325640c4332

Comment 12

[Tim Huang[:timhuang]]

Please reference here [1], and use the module "imgRequest".

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Gecko_Logging

Comment 13

[Dave Huseby [:huseby]]

try looks green to me.  flagging for check in.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32bb28f70092
Test the Isolate the Image Cache per url bar domain. r=baku

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/32bb28f70092

Comment 16

[Thu mua phế liệu]

Công ty phế liệu giá cao Quân Phát đã xem và thấy rất tốt! <a href=" https://phelieuquanphat.com/" rel="nofolow"> https://phelieuquanphat.com/ </a>