Skip to content

Commit

Permalink
style: Preserve CSS input exactly during sanitization.
Browse files Browse the repository at this point in the history
This avoids the mutation due to the different serialization in some cases.

Differential Revision: https://phabricator.services.mozilla.com/D56732
  • Loading branch information
emilio committed Dec 16, 2019
1 parent e8a3a71 commit 02c30bc
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 4 deletions.
1 change: 1 addition & 0 deletions components/style/stylesheets/mod.rs
Expand Up @@ -60,6 +60,7 @@ pub use self::rules_iterator::{NestedRuleIterationCondition, RulesIterator};
pub use self::style_rule::StyleRule;
pub use self::stylesheet::{DocumentStyleSheet, Namespaces, Stylesheet};
pub use self::stylesheet::{StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
pub use self::stylesheet::{SanitizationData, SanitizationKind};
pub use self::supports_rule::SupportsRule;
pub use self::viewport_rule::ViewportRule;

Expand Down
90 changes: 86 additions & 4 deletions components/style/stylesheets/stylesheet.rs
Expand Up @@ -81,6 +81,7 @@ impl StylesheetContents {
quirks_mode: QuirksMode,
line_number_offset: u32,
use_counters: Option<&UseCounters>,
sanitization_data: Option<&mut SanitizationData>,
) -> Self {
let namespaces = RwLock::new(Namespaces::default());
let (rules, source_map_url, source_url) = Stylesheet::parse_rules(
Expand All @@ -94,6 +95,7 @@ impl StylesheetContents {
quirks_mode,
line_number_offset,
use_counters,
sanitization_data,
);

Self {
Expand Down Expand Up @@ -341,6 +343,71 @@ impl StylesheetInDocument for DocumentStyleSheet {
}
}

/// The kind of sanitization to use when parsing a stylesheet.
#[repr(u8)]
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum SanitizationKind {
/// Perform no sanitization.
None,
/// Allow only @font-face, style rules, and @namespace.
Standard,
/// Allow everything but conditional rules.
NoConditionalRules,
}

impl SanitizationKind {
fn allows(self, rule: &CssRule) -> bool {
debug_assert_ne!(self, SanitizationKind::None);
// NOTE(emilio): If this becomes more complex (not filtering just by
// top-level rules), we should thread all the data through nested rules
// and such. But this doesn't seem necessary at the moment.
let is_standard = matches!(self, SanitizationKind::Standard);
match *rule {
CssRule::Document(..) |
CssRule::Media(..) |
CssRule::Supports(..) |
CssRule::Import(..) => false,

CssRule::FontFace(..) |
CssRule::Namespace(..) |
CssRule::Style(..) => true,

CssRule::Keyframes(..) |
CssRule::Page(..) |
CssRule::FontFeatureValues(..) |
CssRule::Viewport(..) |
CssRule::CounterStyle(..) => !is_standard,
}
}
}

/// A struct to hold the data relevant to style sheet sanitization.
#[derive(Debug)]
pub struct SanitizationData {
kind: SanitizationKind,
output: String,
}

impl SanitizationData {
/// Create a new input for sanitization.
#[inline]
pub fn new(kind: SanitizationKind) -> Option<Self> {
if matches!(kind, SanitizationKind::None) {
return None;
}
Some(Self {
kind,
output: String::new(),
})
}

/// Take the sanitized output.
#[inline]
pub fn take(self) -> String {
self.output
}
}

impl Stylesheet {
/// Updates an empty stylesheet from a given string of text.
pub fn update_from_str(
Expand All @@ -365,6 +432,7 @@ impl Stylesheet {
existing.contents.quirks_mode,
line_number_offset,
/* use_counters = */ None,
/* sanitization_data = */ None,
);

*existing.contents.url_data.write() = url_data;
Expand All @@ -391,6 +459,7 @@ impl Stylesheet {
quirks_mode: QuirksMode,
line_number_offset: u32,
use_counters: Option<&UseCounters>,
mut sanitization_data: Option<&mut SanitizationData>,
) -> (Vec<CssRule>, Option<String>, Option<String>) {
let mut rules = Vec::new();
let mut input = ParserInput::new_with_line_number_offset(css, line_number_offset);
Expand Down Expand Up @@ -419,12 +488,24 @@ impl Stylesheet {
{
let mut iter = RuleListParser::new_for_stylesheet(&mut input, rule_parser);

while let Some(result) = iter.next() {
loop {
let rule_start = iter.input.position().byte_index();
let result = match iter.next() {
Some(result) => result,
None => break,
};
match result {
Ok(rule) => {
// Use a fallible push here, and if it fails, just
// fall out of the loop. This will cause the page to
// be shown incorrectly, but it's better than OOMing.
if let Some(ref mut data) = sanitization_data {
if !data.kind.allows(&rule) {
continue;
}
let end = iter.input.position().byte_index();
data.output.push_str(&css[rule_start..end]);
}
// Use a fallible push here, and if it fails, just fall
// out of the loop. This will cause the page to be
// shown incorrectly, but it's better than OOMing.
if rules.try_push(rule).is_err() {
break;
}
Expand Down Expand Up @@ -470,6 +551,7 @@ impl Stylesheet {
quirks_mode,
line_number_offset,
/* use_counters = */ None,
/* sanitized_output = */ None,
);

Stylesheet {
Expand Down

0 comments on commit 02c30bc

Please sign in to comment.