1955775 - nsIPrincipal::GetHashValue can change result over a principal's lifetime.

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Emilio Cobos Álvarez [:emilio]]

This is the root cause of the linked bug:

EnsureHash(https://static01.nyt.com/images/2011/08/04/opinion/BOYLAN_NEW/BOYLAN_NEW-thumbLarge-v6.png): part = 7a7cb03feba0(hash=719293355), loader = 7a7cb03fed60, cd = 0, apptype = 0, cors = 0, result = 3754973933
...
EnsureHash(https://static01.nyt.com/images/2011/08/04/opinion/BOYLAN_NEW/BOYLAN_NEW-thumbLarge-v6.png): part = 7a7cb03feba0(hash=3071997195), loader = 7a7cb03fed60, cd = 0, apptype = 0, cors = 0, result = 288885748

Note how the principal pointer is the same (7a7cb03feba0), yet the hash is different. Nika, Christoph do you know if this is expected? Feels really off...

Comment 1

[Emilio Cobos Álvarez [:emilio]]

I guess this is because of the document.domain setter...

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1955775 - Don't include domain in ContentPrincipal::GetHashValue. r=nika,bholley

As it can mutate over the lifetime of the principal.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49c0ba840ebd
Don't include domain in ContentPrincipal::GetHashValue. r=nika,ckerschb

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/49c0ba840ebd

Comment 5

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #3)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49c0ba840ebd
Don't include domain in ContentPrincipal::GetHashValue. r=nika,ckerschb

Perfherder has detected a browsertime performance change from push 49c0ba840ebdfc5d41e297c8295dc1201de4f696.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
88%	nytimes largestContentfulPaint	macosx1470-64-shippable	fission warm webrender	545.42 -> 68.08	Before/After
86%	nytimes largestContentfulPaint	windows11-64-24h2-shippable	fission warm webrender	389.08 -> 55.65	Before/After
78%	nytimes largestContentfulPaint	linux1804-64-shippable-qr	fission warm webrender	441.80 -> 95.79	Before/After
19%	nytimes ContentfulSpeedIndex	windows11-64-24h2-shippable	fission warm webrender	183.14 -> 148.80	Before/After
18%	nytimes ContentfulSpeedIndex	windows11-64-24h2-shippable	fission warm webrender	187.10 -> 154.35	Before/After
3%	nytimes SpeedIndex	windows11-64-24h2-shippable	fission warm webrender	607.00 -> 591.76	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 44607

The following documentation link provides more information about this command.