Closed Bug 1404678 Opened 2 years ago Closed 2 years ago

stylo: thread '<unnamed>' panicked at 'byte index 2 is not a char boundary; it is inside '−' (bytes 1..4) of `n−1`', src/libcore/str/mod.rs:2161

Categories

(Core :: CSS Parsing and Computation, defect, P2)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: bc, Assigned: xidorn)

References

()

Details

(Keywords: assertion)

Attachments

(3 files)

Attached file Linux Debug Log
1. https://www.nafdigital.no/
   Windows, Linux; Beta/57, Nightly/58

2. thread '<unnamed>' panicked at 'byte index 2 is not a char boundary; it is inside '−' (bytes 1..4) of `n−1`', /checkout/src/libcore/str/mod.rs:2161
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: core::panicking::panic_fmt
   7: core::str::slice_error_fail
   8: cssparser::nth::parse_n_dash_digits
   9: cssparser::nth::parse_nth
  10: selectors::parser::parse_functional_pseudo_class
  11: selectors::parser::parse_one_simple_selector
  12: <selectors::parser::SelectorList<Impl>>::parse
  13: <style::stylesheets::rule_parser::NestedRuleParser<'a, 'b, R>>::parse_nested_rules
  14: <style::stylesheets::rule_parser::NestedRuleParser<'a, 'b, R> as cssparser::rules_and_declarations::AtRuleParser<'i>>::parse_block
  15: Servo_StyleSheet_FromUTF8Bytes
Priority: -- → P2
The https://www.nafdigital.no/assets/build/css/1506686622/main.css sheet is served as text/css (with no charset), but it has a |@charset "UTF-8";| rule at the top.  Looking at the raw bytes of the file, inside the :nth-of-type() selector, at offset 0x396b there are three bytes -- e2 88 92 -- that if decoded as UTF-8 would give a U+2212 MINUS SIGN (where the author should've used a normal HYPHEN-MINUS).

I wonder if this is from bug 1354989.
Should be easy to fix. Taking.
Assignee: nobody → xidorn+moz
Attached file cssparser pull request
This patch is probably not easy to uplift because of the removal of .cargo-ok due to newer version of cargo-vendor.

We may either want to uplift bug 1403407 with this patch, or have someone still have cargo-vendor 0.1.11 installed to generate a patch for uplift.
https://hg.mozilla.org/mozilla-central/rev/ddd5f526cb1f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Attached patch patch for upliftSplinter Review
Approval Request Comment
[Feature/Bug causing the regression]: stylo
[User impact if declined]: may crash in certain cases (e.g. comment 0)
[Is this code covered by automated tests?]: it is covered by unit test of cssparser crate
[Has the fix been verified in Nightly?]: just landed yesterday
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: cssparser 0.12.3 includes two code changes, one is the change for this, and the other is a simple lint fix. The latter is apparently not risky as far as it builds. The change for this bug is not risky either as it just avoids an unnecessary boundary check.
[String changes made/needed]: n/a
Attachment #8914574 - Flags: approval-mozilla-beta?
Comment on attachment 8914574 [details] [diff] [review]
patch for uplift

Stylo related crash fix, beta57+
Attachment #8914574 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7)
> [Is this code covered by automated tests?]: it is covered by unit test of
> cssparser crate
> [Has the fix been verified in Nightly?]: just landed yesterday
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Xidorn's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.