1824325 - Remove support for application/http-index-format Content-Type

Status: RESOLVED FIXED

(Core :: Networking, task, P3)

Description

[Tom Schuster]

While investigating bug 1813066 I realized that the code in nsIndexedToHTML.cpp can be hit my regular web content that uses Content-Type: application/http-index-format. It seems like that is mostly unexpected nowadays and not supported by any other browser.

A search on github reveals some older code using this as well as a lot of CVE reports.

Comment 1

[Tom Schuster]

(We still want to keep support for file:, jar: etc.)

Comment 2

[Tom Schuster]

I had a quick look and it's not obvious to me what the best way of disabling a convert for specific contexts is. Maybe the solution is to just return an error in OnStartRequest and other methods.

Comment 3

[Frederik Braun [:freddy]]

Ed, can you give us some pointers where and how to disable this code for web content (e.g., by looking at the URL scheme).
Would OnStartRequest be appropriate, as Tom theorized?

If so, I think we'd be happy to contribute a small patch.

Comment 4

[edgul]

Had a review of this with Valentin and from what we can tell the only place we spin up a converter from http-index-format is here. So sometime before this call we should be able to check the OriginAttributes for ContentProcess principal. If it is content principal then we treat the input as text, otherwise (file, jar, webextension,etc) we proceed to convert from http-index-format as we currently do.

Hope that helps. Let us know if you need anything else.

Comment 5

[Malte Jürgens [:maltejur]]

Attachment: Bug 1824325 - Remove support for application/http-index-format Content-Type r?freddyb!,#necko-reviewers!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a36520bec3b
Remove support for application/http-index-format Content-Type r=freddyb,necko-reviewers,valentin

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Isn't https://hg.mozilla.org/integration/autoland/rev/3a36520bec3b#l2.13 just useless?

Comment 8

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/3a36520bec3b

Comment 9

[Malte Jürgens [:maltejur]]

Oh yes, you are right. I added that code when trying to figure out why the jar uris were causing assertion failures and forgot to remove it completely again. Thanks for pointing that out.

Comment 10

[Malte Jürgens [:maltejur]]

Attachment: Bug 1824325 - Remove unnescessary code introduced by D198575 r?emilio

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18f5823716e4
Remove unnescessary code introduced by D198575 r=emilio

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/18f5823716e4

Comment 13

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a browsertime performance improvement from either push 3a36520bec3b99a7d0542084bac9df284c1e3b3d or push 4f970ce269e2a101ac3fc0bcec8592fe5a2a8404. I need some help in figuring out which push is more likely to be the cause of the improvement.

== Change summary for alert #41285 (as of Fri, 02 Feb 2024 07:06:37 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
8%	linkedin fcp	linux1804-64-shippable-qr	fission warm webrender	161.05 -> 148.48	Before/After
7%	linkedin FirstVisualChange	linux1804-64-shippable-qr	fission warm webrender	176.78 -> 164.16	Before/After
7%	linkedin FirstVisualChange	linux1804-64-shippable-qr	fission warm webrender	178.75 -> 166.72	Before/After

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=41285.

Comment 14

[Malte Jürgens [:maltejur]]

I don't think this was caused by my changes, it looks to me like this is the result of the fix for Bug 1875265.

Comment 15

[Acasandrei Beatrice (needinfo me)]

Right now some jobs are unable to run and I got confused by that, we'll try to sort this out. Thank you for the input!