Closed Bug 1257902 Opened 4 years ago Closed 4 years ago

Upgrade to Hunspell 1.4.0


(Core :: Spelling checker, defect)

Not set



Tracking Status
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed
firefox49 --- fixed
firefox-esr38 --- wontfix
firefox-esr45 47+ fixed


(Reporter: dveditz, Assigned: RyanVM)



(Keywords: sec-audit, Whiteboard: [post-critsmash-triage][adv-main47+][adv-esr45.2+])


(1 file, 1 obsolete file)

Our current version of Hunspell is 1.3.3 which came out in June 2014. The most recent version 1.3.4 came out in Jan 2016, but includes lots of Coverity-inspired code fixes in late 2014. Some of those are potential security problems.

Note the project now appears to live at, code at
I can take this.
Assignee: nobody → ryanvm
Comment on attachment 8732571 [details] [diff] [review]
update Hunspell to version 1.3.4

Test failures, joy.
Attachment #8732571 - Flags: review?(ehsan)
Group: core-security → dom-core-security
Test appears to be failing in ignore.good, but I'm afraid I'm not in a position to helpfully debug it :\
PROCESS | 17003 | Setting dictionary to ignore
PROCESS | 17003 | getting file by line for file /builds/slave/test/build/tests/xpcshell/tests/extensions/spellcheck/hunspell/tests/unit/data/ignore.good
PROCESS | 17003 | using charset iso-8859-1
PROCESS | 17003 | Expect word example is spelled correctly
TEST-UNEXPECTED-FAIL | extensions/spellcheck/hunspell/tests/unit/test_hunspell.js | do_run_test - [do_run_test : 178] false == true

Should would be nice if the test told us what word it didn't like.
I'm going to skip version 1.3.4 in favor of 1.3.5, which should contain fixes for the various fuzz bugs Tyson has been finding and filing.
Summary: Upgrade to Hunspell 1.3.4 → Upgrade to Hunspell 1.3.5
FYI, I've managed to bisect down to the specific upstream commit that's causing the test failures on Try. No updates on the root cause, though.

I am wondering if it has something to do with our recentish changes to a more-recent upstream SCOWL dictionary update (bug 1137544). Caolan says he's been unable to reproduce the failures when running Hunspell's |make check| locally.
Summary: Upgrade to Hunspell 1.3.5 → Upgrade to Hunspell 1.4.x
I would have expected to fix the problem related to the above commit
I haven't run a new Try push in the last 6 days, so let me try it out and report back!

TEST-PASS | extensions/spellcheck/hunspell/tests/unit/test_hunspell.js | took 34759ms
Tyson, do you want to give another round of fuzzing?
Flags: needinfo?(twsmith)
I've been fuzzing hunspell revision c961e8b7372a1c2a999d216871c965fb8cf843df (2 revisions past 1.4) since it was released and haven't found any issues. I have been fuzzing the sample utf8 dictionary and the sample Korean dictionary.

Caolán has been fixing potential issues faster than I can find them! From a fuzzing perspective this looks good.
Flags: needinfo?(twsmith)
Caolan, I did a build off rev cefc6f2, and spellchecking words with suffixes appears to be broken. Basically, anything plural or otherwise is getting flagged.
Flags: needinfo?(caolanm)
Oh, maybe tied to the affixmgr .rej files that got checked into the tree recently? Unresolved conflict?
what exactly doesn't work, give an example ? I presume this is with the mozilla en_US dictionary ?
Flags: needinfo?(caolanm)
Correct, Fx browser build. I pushed it to Try if you want to play with the resulting builds yourself.
Also, Android builds are busted:
/builds/slave/try-and-api-15-000000000000000/build/src/extensions/spellcheck/hunspell/src/hunzip.hxx:85:45: error: passing 'const ifstream {aka const std::basic_ifstream<char, std::char_traits<char> >}' as 'this' argument of 'bool std::basic_ifstream<_CharT, _Traits>::is_open() [with _CharT = char; _Traits = std::char_traits<char>]' discards qualifiers [-fpermissive]
Unsurprisingly, this is failing tests too:

PROCESS | 17150 | Expect word try is spelled correctly
TEST-PASS | extensions/spellcheck/hunspell/tests/unit/test_hunspell.js | do_run_test - [do_run_test : 178] true == true
PROCESS | 17150 | Expect word tried is spelled correctly
TEST-UNEXPECTED-FAIL | extensions/spellcheck/hunspell/tests/unit/test_hunspell.js | do_run_test - [do_run_test : 178] false == true
I can see the failure in your build, but if I try and build my own by
hg clone
and copying the .cxxs and hxxs of cefc6f2 over the ones in that tree it results in an apparently working spellchecking experience so I can't debug it. A bit odd.
The final 1.4.0 release passes tests on Try and contains all the relevant fuzzer fixes, so let's just go with it. There's a lot of upstream refactoring going on, so I expect the next release to be a ways out still.
Summary: Upgrade to Hunspell 1.4.x → Upgrade to Hunspell 1.4.0
Ehsan, I know you're busy and not working on Editor anymore, but I really have no clue who else to ask to review this. Feel free to redirect it to someone more appropriate if there is one.

Just one note, this drops the patch from bug 983817, which never got upstreamed. I asked Caolan about it and he said that Hunspell is using std::sort now, so it's very unlikely to be an issue anymore. But we might want to keep an eye on crash-stats for any similar-looking signatures and re-add the patch if needed.
Attachment #8732571 - Attachment is obsolete: true
Attachment #8746910 - Flags: review?(ehsan)
Attachment #8746910 - Flags: review?(ehsan) → review+

I'm going to let this bake on Nightly for a few days and have my Softvision team give this a QA pass, then I'll request uplift to the various release branches.
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Blocks: 1269941
Group: dom-core-security → core-security-release
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main47+][adv-esr45.2+]
Group: core-security-release
Depends on: 1322666
You need to log in before you can comment on or make changes to this bug.