Closed Bug 1299267 Opened 3 years ago Closed 3 years ago

Return a network error for requests whose type is "script" with additional bad MIME types

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [domsecurity-backlog1])

Attachments

(3 files, 1 obsolete file)

We are pretty close to landing a pull request that also blocks audio/*, video/* and text/csv.
Assignee: evilpies → nobody
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Assignee: nobody → evilpies
Attachment #8787996 - Flags: review?(ckerschb)
I really wanted to use add_task, but for some reason that wasn't available. The new test set-up makes it easy to add new mime types for testing.
Attachment #8787997 - Flags: review?(ckerschb)
Attachment #8787996 - Attachment is obsolete: true
Attachment #8787996 - Flags: review?(ckerschb)
Attachment #8788152 - Flags: review?(ckerschb)
Comment on attachment 8788152 [details] [diff] [review]
Block additional wrong mime types for scripts

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

Thanks, r=me;

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +1070,4 @@
>          return NS_OK;
>      }
>  
> +    bool block = true;

Nit: I would prefer if we do |bool block = false;| and within every branch explicitly update to |block = true;| and remove the else branch, but I leave it up to you.
Attachment #8788152 - Flags: review?(ckerschb) → review+
Comment on attachment 8787997 [details] [diff] [review]
Test for wrong mime types

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

thanks, r=me
Attachment #8787997 - Flags: review?(ckerschb) → review+
We still have around 5% of unknown types, especially the empty case should reduce this.
Attachment #8788175 - Flags: review?(ckerschb)
Comment on attachment 8788175 [details] [diff] [review]
Add additonal telemetry types

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

thanks. r=me

::: toolkit/components/telemetry/Histograms.json
@@ +5527,4 @@
>      "bug_numbers": [1288361],
>      "expires_in_version": "56",
>      "kind": "enumerated",
> +    "n_values": 15,

I am not entirely sure about the relation between n_values and the actual values. I would imagine you want n_values to be 12 though.
Attachment #8788175 - Flags: review?(ckerschb) → review+
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)
> ::: toolkit/components/telemetry/Histograms.json
> @@ +5527,4 @@
> >      "bug_numbers": [1288361],

Actually, I think you should add 1299267 to the bug_numbers.
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb31c2fb7a7d
Block additional wrong mime types for scripts. r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f81dca324d1
Test for wrong mime types. r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e802eac60df
Add additonal telemetry types. r=ckerschb
Keywords: dev-doc-needed
Seems like I screwed up the telemetry data :/ https://mzl.la/2bQOUXZ Somehow we get a huge amount of hits on 11/12? I only added 10/11.
Blocks: 1300891
No longer blocks: 1300891
Depends on: 1300891
Blocks: 1333995
Duplicate of this bug: 1235872
You need to log in before you can comment on or make changes to this bug.