Update Graphite2 library to 1.3.8

VERIFIED FIXED in Firefox 46

Status

()

enhancement
VERIFIED FIXED
3 years ago
a month ago

People

(Reporter: Virtual, Assigned: jfkthame)

Tracking

({nightly-community})

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 wontfix, firefox46+ verified, firefox47+ verified, firefox48+ verified, firefox-esr3846+ verified, firefox-esr4546+ verified)

Details

()

Attachments

(2 attachments)

> Changelog:
>    This release fixes a few feature bugs particular for collision avoidance.
>    It also fixes a few fuzz bugs, etc.
Assignee

Comment 1

3 years ago
This fixes a bunch of recently-found fuzz bugs.
Attachment #8739103 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8739103 - Flags: review?(jmuizelaar) → review+

Comment 3

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d58d61d36ee9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Do we want this on branches?
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
I believe we do not want this on ESR45 since we disable Graphite library on that version. I think we should uplift to Aurora 47. Hi Jonathan, could you please request uplift?
Flags: needinfo?(rkothari) → needinfo?(jfkthame)
(In reply to Ritu Kothari (:ritu) from comment #5)
> I believe we do not want this on ESR45 since we disable Graphite library on
> that version. I think we should uplift to Aurora 47. Hi Jonathan, could you
> please request uplift?

It's not my call here but I just want to point out that this patch is almost entirely security fixes for issues that I and others have found through fuzzing. Patches for these bugs are already public and can be used to locate the vulnerabilities. Unless we are not building graphite in to ESR45, which I believe we are, we should patch it. Just in case we (or someone else) decide to re-enable it.
Assignee

Comment 7

3 years ago
Comment on attachment 8739103 [details] [diff] [review]
Update graphite2 library to release 1.3.8

Approval Request Comment
[Feature/regressing bug #]: graphite2 text shaping lib

[User impact if declined]: various vulnerabilities (possible crashes, undefined behavior, ...) when graphite is preffed on

[Describe test coverage new/current, TreeHerder]: our existing reftests check that basic functionality is working within gecko; more extensive unit testing of graphite features happens upstream, and fuzz-testing of the standalone library is ongoing.

[Risks and why]: minimal risk - collection of generally simple bugfixes (arithmetic overflows, null-checks, bounds-checks, etc)

[String/UUID change made/needed]: none


As Tyson notes, this code is present in esr45, though preffed off by default at runtime. Because it's preffed off anyway, there's no stability risk in updating it for esr users in general; but for the sake of any who do turn it on (or if we want to re-enable it by default during the lifetime of esr45, perhaps at the same time as we re-enable it for the regular Firefox release channel), I think we should consider updating the library there too.
Flags: needinfo?(jfkthame)
Attachment #8739103 - Flags: approval-mozilla-esr45?
Attachment #8739103 - Flags: approval-mozilla-aurora?
We could uplift this to beta, but it is a little bit late in the cycle to find and respond to any issues that come up. We are turning the graphite library back on by default in 46. If we take this update in 46 we may want to disable it by default. Jonathan, what do you think about the risk here?
Flags: needinfo?(lhenry) → needinfo?(jfkthame)
Assignee

Comment 9

3 years ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> We could uplift this to beta, but it is a little bit late in the cycle to
> find and respond to any issues that come up. We are turning the graphite
> library back on by default in 46.

Are we really re-enabling graphite in 46? As things stand, the defaults currently in mozilla-beta/.../all.js will keep it off; I'd assumed we would leave it off-by-default at least until 47, given that we're just landing this collection of fixes now.

Things are definitely much more stable than a few weeks ago, but IMO holding off for another cycle would be reasonable, in terms of balancing the risk of remaining issues vs the inconvenience to the users who need it.

> If we take this update in 46 we may want
> to disable it by default. Jonathan, what do you think about the risk here?

I think the risk is pretty minimal (even if we do re-enable by default), given the nature of the fixes here; but my personal recommendation would be to take the more-cautious approach of landing the library update but keeping it disabled-by-default for 46, so that only the (small minority of) users who explicitly re-enable will be affected at all.
Flags: needinfo?(jfkthame)
OK, that sounds good to me. I just double checked and you are right, it's still disabled for release.
Comment on attachment 8739103 [details] [diff] [review]
Update graphite2 library to release 1.3.8

Let's update graphite on aurora and beta. This should make it into beta 11.
Attachment #8739103 - Flags: approval-mozilla-beta+
Attachment #8739103 - Flags: approval-mozilla-aurora?
Attachment #8739103 - Flags: approval-mozilla-aurora+
Comment on attachment 8739103 [details] [diff] [review]
Update graphite2 library to release 1.3.8

Just like beta, taking it in both 38 & 45.
Attachment #8739103 - Flags: approval-mozilla-esr45?
Attachment #8739103 - Flags: approval-mozilla-esr45+
Attachment #8739103 - Flags: approval-mozilla-esr38+
Assignee

Comment 14

3 years ago
We'll actually need an updated patch for the ESR trees, as they're (both) currently on 1.3.6 (in effect, by taking the update to 1.3.8 here we're also taking the 1.3.7 update from bug 1255158). I'll post the ESR patch shortly, after checking it builds OK locally.
Assignee

Comment 15

3 years ago
This is the version of the graphite-1.3.8 patch for esr38 and esr45 trees.
Assignee

Comment 16

3 years ago
https://hg.mozilla.org/releases/mozilla-esr45/rev/7cc1fb0ef533b24a0b63075ae5b7efe3ca655d26
Bug 1262846 (patch for ESR trees) - Update Graphite2 library to 1.3.8. r=jrmuizel a=sledru
You need to log in before you can comment on or make changes to this bug.