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

Status: RESOLVED FIXED

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

Description

[Jesse Schwartzentruber (:truber)]

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

Comment 1

[Jesse Schwartzentruber (:truber)]

Attachment: testcase.html

Comment 2

[Emilio Cobos Álvarez (:emilio)]

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!

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Hmm... Maybe it's not the same issue, hold on :)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah, it is, never mind me.

Comment 5

[Simon Sapin (:SimonSapin)]

Right, it’s similar but not exactly the same (and not fixed by cssparser#175). I’m working on a fix.

Comment 6

[Simon Sapin (:SimonSapin)]

Attachment: Bug 1383975 - Update cssparser, fix a panic in bad-url token parsing.

Review commit: https://reviewboard.mozilla.org/r/160944/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/160944/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

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

https://reviewboard.mozilla.org/r/160944/#review166250

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3fa44b134c34
Update cssparser, fix a panic in bad-url token parsing. r=emilio

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3fa44b134c34

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 11

[Bobby Holley (:bholley)]

We should land the crashtest.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5697f69b1426
Add crashtest.

Comment 13

[Bobby Holley (:bholley)]

Thanks Ryan.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5697f69b1426