Closed Bug 1084026 Opened 5 years ago Closed 5 years ago

enable WOFF2 by default in release builds

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed
relnote-firefox --- 39+

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

No description provided.
Support for WOFF2 was landed behind a runtime pref in bug 1064737. Before enabling it by default in release, we probably want to do some security review/testing of the WOFF2 decoder and Brotli decompression code.
Flags: sec-review?
Release Note Request (optional, but appreciated)
[Why is this notable]: This is currently only behind a pref for aurora/nightly (see bug 1064737) so want to make sure we note when this moves to release.
[Suggested wording]: OFF2 fonts are now supported
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
That should read
> [Suggested wording]: WOFF2 fonts are now supported
Is there any update here?  When will this move to release channels? Has it had security review/testing?
Flags: needinfo?(jfkthame)
I'd like to get this enabled on release channels sometime soon; we've had it on nightly/aurora since mozilla-35, and would like to unblock it and let it ride the train to release (for mozilla-38, maybe?).

:dveditz, what should we be doing w.r.t. security review for this? The feature exposes new code (the WOFF2 decoder, including the Brotli decompressor) to potentially malicious code in the form of @font-face resources. The code we're using is Google's implementation, which they've been shipping since Chrome 36, so it has already been exposed (and reviewed) on their side, but I wondered how much additional review/testing we might feel is needed?
Flags: sec-review?(dveditz)
Flags: sec-review?
Flags: needinfo?(jfkthame)
The OTS WOFF2 decoder does not seem to be in fully conformance with the spec yet (see https://github.com/google/woff2/pull/3), so I’d rather prefer Firefox to wait until the reference implementation is checked against the current spec.
(In reply to Khaled Hosny from comment #6)
> The OTS WOFF2 decoder does not seem to be in fully conformance with the spec
> yet (see https://github.com/google/woff2/pull/3), so I’d rather prefer
> Firefox to wait until the reference implementation is checked against the
> current spec.

That's a fair point, but I don't think it should deter us from moving forward with any security review and testing that is deemed necessary. (In particular, it doesn't relate to the Brotli decoder code, which is a substantial part of what's involved here.)
+cc: Jesse. Would your team want to fuzz-test and/or review this code from Google before we ship it?
Flags: needinfo?(jruderman)
Flags: needinfo?(jruderman) → needinfo?(cdiehl)
Depends on: 1142952
Depends on: 1142953
We should be fine here as long as
 * we keep up with Google's WOFF2/Brotli code
 * we keep up with Google's OTS code
 * we fuzz it to shake out problems in the interface between our code and theirs.

It wouldn't hurt to really beat on it in case our fuzzers exercise different paths than Google's (and Google bounty seekers), but given the fuzzing resources they've no doubt already put into this code it's better to focus the bulk of our fuzzing efforts on other features where's we've written the code ourselves or used libraries from other sources.

If we have a corpus of WOFF2 fonts it shouldn't be hard to adapt the original WOFF fuzzer to exercise this code. Run it for a bit and we should quickly see if it needs lots more testing or if it's pretty solid.
Flags: sec-review?(dveditz) → sec-review+
Depends on: 1143045
Here's the (trivial) patch to enable WOFF2 across all release channels, for when we're ready to land it.
Attachment #8577359 - Flags: review?(jdaggett)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8577359 - Flags: review?(jdaggett) → review+
Jonathan, do you have an update on when we ship that? Thanks
Flags: needinfo?(jfkthame)
I'm intending to land the patch this week, so it should go into FF39. (Just waiting for inbound to re-open...)
Flags: needinfo?(jfkthame)
OK. Thanks!
Target Milestone: --- → mozilla39
https://hg.mozilla.org/mozilla-central/rev/97d1d0f97785
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Adding this as a release note for 39 in aurora. Here is the current wording and the linked MDN page:

[WOFF2][1] (Web Open Font Format) fonts are now supported

  [1]: https://developer.mozilla.org/en-US/docs/Web/Guide/WOFF

Please needinfo me if you have suggested changes.
(In reply to Liz Henry (:lizzard) from comment #16)
> Adding this as a release note for 39 in aurora. Here is the current wording
> and the linked MDN page:
> 
> [WOFF2][1] (Web Open Font Format) fonts are now supported

Probably should make that "Web Open Font Format 2.0".

> 
>   [1]: https://developer.mozilla.org/en-US/docs/Web/Guide/WOFF
> 
> Please needinfo me if you have suggested changes.


It's not totally clear to me that it makes sense to relnote this for Aurora 39, given that we've been shipping with WOFF2.0 enabled on nightly/aurora channels for several versions already. What's new here is that we're turning it on for beta/release, but that means the change doesn't really have any effect until FF39 hits the Beta channel.
Flags: needinfo?(lhenry)
Thanks, I missed that. I'll wait to note it till beta, then!
Flags: needinfo?(lhenry)
Flags: needinfo?(cdiehl)
You need to log in before you can comment on or make changes to this bug.