Closed Bug 1824325 Opened 1 year ago Closed 5 months ago

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

Categories

(Core :: Networking, task, P3)

task

Tracking

()

RESOLVED FIXED
124 Branch
Tracking Status
firefox124 --- fixed

People

(Reporter: tschuster, Assigned: mjurgens)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

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.

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

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.

Severity: -- → N/A
Priority: -- → P3
Whiteboard: [necko-triaged]
Blocks: 1647898

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.

Flags: needinfo?(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.

Flags: needinfo?(edgul) → needinfo?(fbraun)
See Also: → 1871048
Assignee: nobody → mjurgens
Status: NEW → ASSIGNED
No longer blocks: 1647898
Depends on: 1875001
Blocks: 1875001
No longer depends on: 1875001
Depends on: 1875068
Pushed by mjurgens@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a36520bec3b
Remove support for application/http-index-format Content-Type r=freddyb,necko-reviewers,valentin
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 124 Branch

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.

Status: RESOLVED → REOPENED
Flags: needinfo?(mjurgens)
Resolution: FIXED → ---
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/18f5823716e4
Remove unnescessary code introduced by D198575 r=emilio
Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED

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.

Flags: needinfo?(mjurgens)

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.

Flags: needinfo?(mjurgens) → needinfo?(bacasandrei)

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!

Flags: needinfo?(bacasandrei)
Flags: needinfo?(fbraun)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: