Implemementing context-based MIME type sniffing #8190

jdm · 2015-10-25T15:56:36Z

This is a rebase of #7842 that also adds a test.
Fixes #4183.

@Yoric, how's this look to you?

jdm · 2015-10-25T15:58:02Z

r? @glennw for the font-loading changes.

frewsxcv · 2015-10-25T15:58:41Z

components/gfx/font_cache_task.rs

+                                            metadata.content_type.as_ref().map(|content_type| {
+                                                let mime = &content_type.0;
+                                                is_supported_font_type(&mime.0, &mime.1)
+                                            }).unwrap_or(false);


map(..).unwrap_or(..) can be simplified to .map_or(..)

eefriedman · 2015-10-25T19:01:43Z

components/net/mime_classifier.rs

-                                Some(MediaType::AudioVideo) => self.audio_video_classifier.classify(data),
-                                None => None
-                            }.unwrap_or(supplied_type.clone())
+                    data: &[u8]) -> Option<(String, String)> {


Is there some reason you're changing this? It will substantially simplify downstream code if we can assume MIME sniffing always returns something (even if it's just application/octet-stream or something).

That's a good point; I didn't consider just turning a missing Content-Type and unclassifiable into application/octet-stream.

bors-servo · 2015-10-26T21:33:55Z

☔ The latest upstream changes (presumably #7979) made this pull request unmergeable. Please resolve the merge conflicts.

glennw · 2015-11-08T23:28:24Z

@jdm Sorry, I missed this one - is it waiting on my review now, or are other changes required before it is worth looking at it?

jdm · 2015-11-09T13:31:06Z

I don't think the requested changes have any effect on the stuff you're reviewing.

glennw · 2015-11-12T06:26:34Z

@jdm The font load changes look ok to me, apart from the question about the mutex usage.

jdm · 2015-11-12T19:09:49Z

r? @eefriedman for the most recent changes.

eefriedman · 2015-11-12T19:42:59Z

I'd also like to see a test to make sure that content served as application/xhtml+xml is never sniffed as a font, even if it is one.

eefriedman · 2015-11-12T20:04:44Z

Review status: 0 of 25 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

^{components/gfx/font_cache_task.rs, line 381 [r7] (raw file):}
x-font-ttf etc. will become irrelevant once sniffing is on by default; maybe put in a FIXME?

^{components/net/mime_classifier.rs, line 46 [r7] (raw file):}
Option has a cloned() method.

^{components/net/mime_classifier.rs, line 235 [r7] (raw file):}
Extra newline.

^{components/script/document_loader.rs, line 38 [r7] (raw file):}
Space before "|".

Comments from the review on Reviewable.io

bors-servo · 2015-11-13T17:01:47Z

☔ The latest upstream changes (presumably #6935) made this pull request unmergeable. Please resolve the merge conflicts.

Yoric · 2015-11-25T10:56:22Z

I realize I never responded. @jdm, yes, this lgtm.


        Resolves #4183 - Implemementing context-based MIME type sniffing

The version of the standard is not finalized at the time of this writing. Specifications may be found here: https://mimesniff.spec.whatwg.org/#context-specific-sniffing .


        Restrict font loads to known MIME types.


        Use valid MIME types in the font classifier.


        Enable MIME sniffing via a pref instead of a global option.


        Add a test for contextual MIME sniffing.


        Remove unneeded dead_code annotations.


        Send a response to font load initiators even when a font is ignored.


        Update mime_guess to incorporate TTF MIME changes.

jdm · 2015-12-31T12:46:30Z

@bors-servo: try

bors-servo · 2015-12-31T12:46:31Z

⌛ Trying commit bda341b with merge 4092a8d...


        Auto merge of #8190 - jdm:4138, r=<try>

@Yoric

Implemementing context-based MIME type sniffing This is a rebase of #7842 that also adds a test. Fixes #4183. @Yoric, how's this look to you?  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8190)

bors-servo · 2015-12-31T13:22:39Z

💔 Test failed - linux-rel

nox · 2015-12-31T13:24:11Z

Tests with unexpected results:
  ▶ FAIL [expected PASS] /_mozilla/css/octicons_a.html
  └   → /_mozilla/css/octicons_a.html f03141f6c2d6c4dc01770f2eac9356912b711b30
/_mozilla/css/octicons_ref.html f03141f6c2d6c4dc01770f2eac9356912b711b30
Testing f03141f6c2d6c4dc01770f2eac9356912b711b30 != f03141f6c2d6c4dc01770f2eac9356912b711b30

  ▶ FAIL [expected PASS] /_mozilla/css/ol_japanese_iroha_bullet_styles.html
  └   → /_mozilla/css/ol_japanese_iroha_bullet_styles.html ff6a9a084545fcff9b7b3584c2070fb986e5cc7b
/_mozilla/css/ol_japanese_iroha_ref.html ff6a9a084545fcff9b7b3584c2070fb986e5cc7b
Testing ff6a9a084545fcff9b7b3584c2070fb986e5cc7b != ff6a9a084545fcff9b7b3584c2070fb986e5cc7b

Tests with unexpected results:
  ▶ FAIL [expected PASS] /css-variables-1_dev/html/variable-font-face-02.htm
  └   → /css-variables-1_dev/html/variable-font-face-02.htm a79786e29fec1999707ffcdcf135997be11e440e
/css-variables-1_dev/html/reference/variable-font-face-02-ref.htm 6a5386967311195847b410a07bbfc3ecdfe87e22
Testing a79786e29fec1999707ffcdcf135997be11e440e == 6a5386967311195847b410a07bbfc3ecdfe87e22

  ▶ FAIL [expected PASS] /css-variables-1_dev/html/variable-external-font-face-01.htm
  └   → /css-variables-1_dev/html/variable-external-font-face-01.htm a79786e29fec1999707ffcdcf135997be11e440e
/css-variables-1_dev/html/reference/variable-external-font-face-01-ref.htm 6a5386967311195847b410a07bbfc3ecdfe87e22
Testing a79786e29fec1999707ffcdcf135997be11e440e == 6a5386967311195847b410a07bbfc3ecdfe87e22

  ▶ FAIL [expected PASS] /css-variables-1_dev/html/variable-font-face-01.htm
  └   → /css-variables-1_dev/html/variable-font-face-01.htm a79786e29fec1999707ffcdcf135997be11e440e
/css-variables-1_dev/html/reference/variable-font-face-01-ref.htm 6a5386967311195847b410a07bbfc3ecdfe87e22
Testing a79786e29fec1999707ffcdcf135997be11e440e == 6a5386967311195847b410a07bbfc3ecdfe87e22

  ▶ PASS [expected FAIL] /css21_dev/html4/floats-wrap-bfc-002-right-table.htm


        Accept all Content-Type values for fonts if sniffing is disabled. Res…

…tore the status quo.

jdm · 2015-12-31T14:13:53Z

There. All tests pass locally for me now.

nox · 2015-12-31T15:27:01Z

@bors-servo r=eefriedman

bors-servo · 2015-12-31T15:27:02Z

📌 Commit 20668da has been approved by eefriedman

bors-servo · 2015-12-31T15:43:11Z

⌛ Testing commit 20668da with merge af1690f...


        Auto merge of #8190 - jdm:4138, r=eefriedman

@Yoric

Implemementing context-based MIME type sniffing This is a rebase of #7842 that also adds a test. Fixes #4183. @Yoric, how's this look to you?  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8190)

bors-servo · 2015-12-31T16:20:21Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review label Oct 25, 2015

jdm mentioned this pull request Oct 25, 2015

gfx: Implement WOFF support in Servo via conversion to OTF with the `rust-woff` crate. #7349

Closed

highfive assigned glennw Oct 25, 2015

frewsxcv reviewed Oct 25, 2015
View changes

jdm mentioned this pull request Oct 25, 2015

Resolves #4183 - Implemementing context-based MIME type sniffing #7842

Closed

eefriedman reviewed Oct 25, 2015
View changes

highfive added S-needs-rebase and removed S-awaiting-merge labels Oct 26, 2015

jdm force-pushed the jdm:4138 branch from f552b23 to 743117c Nov 12, 2015

highfive assigned eefriedman and unassigned glennw Nov 12, 2015

jdm removed the S-needs-rebase label Nov 12, 2015

jdm added S-needs-code-changes and removed S-awaiting-review labels Nov 12, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Nov 13, 2015

jdm force-pushed the jdm:4138 branch from 743117c to 4bc9066 Nov 25, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 25, 2015

jdm removed the S-needs-rebase label Nov 25, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Dec 31, 2015

Yoric and others added 8 commits Sep 30, 2015

Resolves #4183 - Implemementing context-based MIME type sniffing

1e81b8c

The version of the standard is not finalized at the time of this writing. Specifications may be found here: https://mimesniff.spec.whatwg.org/#context-specific-sniffing .

Restrict font loads to known MIME types.

e17e553

Use valid MIME types in the font classifier.

9601d55

Enable MIME sniffing via a pref instead of a global option.

cca25e2

Add a test for contextual MIME sniffing.

6ba66f4

Remove unneeded dead_code annotations.

3d8881c

Send a response to font load initiators even when a font is ignored.

afb4d90

Update mime_guess to incorporate TTF MIME changes.

Loading status checks…

bda341b

jdm force-pushed the jdm:4138 branch from d60d4b3 to bda341b Dec 31, 2015

jdm removed the S-needs-rebase label Dec 31, 2015

highfive added the S-tests-failed label Dec 31, 2015

Accept all Content-Type values for fonts if sniffing is disabled. Res…

Loading status checks…

20668da

…tore the status quo.

highfive removed the S-tests-failed label Dec 31, 2015

highfive added S-awaiting-merge and removed S-awaiting-review labels Dec 31, 2015

bors-servo mentioned this pull request Dec 31, 2015

Moving the error handling out of network loader... #8851

Closed

servo / servo

Implemementing context-based MIME type sniffing #8190

Implemementing context-based MIME type sniffing #8190

jdm commented Oct 25, 2015

jdm commented Oct 25, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Oct 26, 2015

glennw commented Nov 8, 2015

jdm commented Nov 9, 2015

glennw commented Nov 12, 2015

jdm commented Nov 12, 2015

eefriedman commented Nov 12, 2015

eefriedman commented Nov 12, 2015

bors-servo commented Nov 13, 2015

Yoric commented Nov 25, 2015

jdm commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015

nox commented Dec 31, 2015

jdm commented Dec 31, 2015

nox commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015

servo / servo

Join GitHub today

Implemementing context-based MIME type sniffing #8190

Implemementing context-based MIME type sniffing #8190

Conversation

jdm commented Oct 25, 2015

jdm commented Oct 25, 2015

This comment has been minimized.

frewsxcv Oct 25, 2015

This comment has been minimized.

eefriedman Oct 25, 2015

This comment has been minimized.

jdm Oct 25, 2015

bors-servo commented Oct 26, 2015

glennw commented Nov 8, 2015

jdm commented Nov 9, 2015

glennw commented Nov 12, 2015

jdm commented Nov 12, 2015

eefriedman commented Nov 12, 2015

eefriedman commented Nov 12, 2015

bors-servo commented Nov 13, 2015

Yoric commented Nov 25, 2015

jdm commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015

nox commented Dec 31, 2015

jdm commented Dec 31, 2015

nox commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015

bors-servo commented Dec 31, 2015