stylo: panicked at 'byte index 15 is not a char boundary; it is inside '...

RESOLVED FIXED in Firefox 56

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: truber, Assigned: SimonSapin)

Tracking

(Blocks 2 bugs, {assertion, testcase})

Trunk
mozilla56
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 fixed)

Details

Attachments

(2 attachments)

The attached testcase causes a panic in m-c rev dcfb58fcb6dd with stylo enabled by pref.

thread '<unnamed>' panicked at 'byte index 15 is not a char boundary; it is inside '۰' (bytes 14..16) of `
color: url(9
۰
`', /checkout/src/libcore/str/mod.rs:2179
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::begin_panic
   6: std::panicking::begin_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::str::slice_error_fail
  10: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::Range<usize>>::index::{{closure}}
  11: <core::option::Option<T>>::unwrap_or_else
  12: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::Range<usize>>::index
  13: core::str::traits::<impl core::ops::Index<core::ops::Range<usize>> for str>::index
  14: cssparser::tokenizer::Tokenizer::slice_from
  15: cssparser::tokenizer::consume_unquoted_url::consume_bad_url
  16: cssparser::tokenizer::consume_unquoted_url::consume_url_end
  17: cssparser::tokenizer::consume_unquoted_url::consume_unquoted_url_internal
  18: cssparser::tokenizer::consume_unquoted_url
  19: cssparser::tokenizer::consume_ident_like
  20: cssparser::tokenizer::next_token
  21: cssparser::tokenizer::Tokenizer::next
  22: cssparser::parser::parse_until_before
  23: cssparser::parser::Parser::parse_until_before
  24: cssparser::parser::Parser::parse_comma_separated
  25: <selectors::parser::SelectorList<Impl>>::parse
  26: <style::stylesheets::rule_parser::NestedRuleParser<'a, 'b> as cssparser::rules_and_declarations::QualifiedRuleParser<'i>>::parse_prelude
  27: <style::stylesheets::rule_parser::TopLevelRuleParser<'a> as cssparser::rules_and_declarations::QualifiedRuleParser<'i>>::parse_prelude
  28: cssparser::rules_and_declarations::parse_qualified_rule::{{closure}}
  29: cssparser::parser::Parser::parse_entirely
  30: cssparser::parser::parse_until_before
  31: cssparser::rules_and_declarations::parse_qualified_rule
  32: <cssparser::rules_and_declarations::RuleListParser<'i, 't, 'a, P> as core::iter::iterator::Iterator>::next
  33: style::stylesheets::stylesheet::Stylesheet::parse_rules
  34: style::stylesheets::stylesheet::StylesheetContents::from_str
  35: Servo_StyleSheet_FromUTF8Bytes
Flags: in-testsuite?
Posted file testcase.html
This is https://github.com/servo/rust-cssparser/issues/174, which is fixed in https://github.com/servo/rust-cssparser/pull/175. Now we need to pull that in m-c, thanks for the catch Jesse!
Hmm... Maybe it's not the same issue, hold on :)
Yeah, it is, never mind me.
Assignee

Comment 5

2 years ago
Right, it’s similar but not exactly the same (and not fixed by cssparser#175). I’m working on a fix.
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8889878 [details]
Bug 1383975 - Update cssparser, fix a panic in bad-url token parsing.

https://reviewboard.mozilla.org/r/160944/#review166250
Attachment #8889878 - Flags: review?(emilio+bugs) → review+

Comment 8

2 years ago
Pushed by simon.sapin@exyr.org:
https://hg.mozilla.org/integration/autoland/rev/3fa44b134c34
Update cssparser, fix a panic in bad-url token parsing. r=emilio
Priority: -- → P1

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3fa44b134c34
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Are the tests.rs changes in this patch sufficient testing for this issue or should we land the attached testcase as a crashtest as well?
Assignee: nobody → simon.sapin
Flags: needinfo?(simon.sapin)
We should land the crashtest.
Thanks Ryan.
Flags: needinfo?(simon.sapin)
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.