Skip to content

Commit

Permalink
style: Prevent exponential blowup of custom properties.
Browse files Browse the repository at this point in the history
Put a hard cap on the value length instead of counting substitutions, because it
works best, see the comment.

Differential Revision: https://phabricator.services.mozilla.com/D13352
  • Loading branch information
emilio committed Dec 2, 2018
1 parent f6b6825 commit dca315e
Showing 1 changed file with 44 additions and 16 deletions.
60 changes: 44 additions & 16 deletions components/style/custom_properties.rs
Expand Up @@ -283,18 +283,32 @@ impl VariableValue {
}
}

fn push(
fn push<'i>(
&mut self,
input: &Parser<'i, '_>,
css: &str,
css_first_token_type: TokenSerializationType,
css_last_token_type: TokenSerializationType,
) {
) -> Result<(), ParseError<'i>> {
/// Prevent values from getting terribly big since you can use custom
/// properties exponentially.
///
/// This number (1MB) is somewhat arbitrary, but silly enough that no
/// sane page would hit it. We could limit by number of total
/// substitutions, but that was very easy to work around in practice
/// (just choose a larger initial value and boom).
const MAX_VALUE_LENGTH_IN_BYTES: usize = 1024 * 1024;

if self.css.len() + css.len() > MAX_VALUE_LENGTH_IN_BYTES {
return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
}

// This happens e.g. between two subsequent var() functions:
// `var(--a)var(--b)`.
//
// In that case, css_*_token_type is nonsensical.
if css.is_empty() {
return;
return Ok(());
}

self.first_token_type.set_if_nothing(css_first_token_type);
Expand All @@ -307,21 +321,32 @@ impl VariableValue {
self.css.push_str("/**/")
}
self.css.push_str(css);
self.last_token_type = css_last_token_type
self.last_token_type = css_last_token_type;
Ok(())
}

fn push_from(
fn push_from<'i>(
&mut self,
input: &Parser<'i, '_>,
position: (SourcePosition, TokenSerializationType),
input: &Parser,
last_token_type: TokenSerializationType,
) {
self.push(input.slice_from(position.0), position.1, last_token_type)
) -> Result<(), ParseError<'i>> {
self.push(
input,
input.slice_from(position.0),
position.1,
last_token_type,
)
}

fn push_variable(&mut self, variable: &ComputedValue) {
fn push_variable<'i>(
&mut self,
input: &Parser<'i, '_>,
variable: &ComputedValue,
) -> Result<(), ParseError<'i>> {
debug_assert!(variable.references.is_empty());
self.push(
input,
&variable.css,
variable.first_token_type,
variable.last_token_type,
Expand Down Expand Up @@ -727,6 +752,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment:
// title=Tarjan%27s_strongly_connected_components_algorithm&oldid=801728495

/// Struct recording necessary information for each variable.
#[derive(Debug)]
struct VarInfo {
/// The name of the variable. It will be taken to save addref
/// when the corresponding variable is popped from the stack.
Expand All @@ -741,6 +767,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment:
}
/// Context struct for traversing the variable graph, so that we can
/// avoid referencing all the fields multiple times.
#[derive(Debug)]
struct Context<'a> {
/// Number of variables visited. This is used as the order index
/// when we visit a new unresolved variable.
Expand Down Expand Up @@ -941,7 +968,7 @@ fn substitute_references_in_value<'i>(
environment,
)?;

computed_value.push_from(position, &input, last_token_type);
computed_value.push_from(&input, position, last_token_type)?;
Ok(computed_value)
}

Expand All @@ -955,8 +982,8 @@ fn substitute_references_in_value<'i>(
///
/// Return `Err(())` if `input` is invalid at computed-value time.
/// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise.
fn substitute_block<'i, 't>(
input: &mut Parser<'i, 't>,
fn substitute_block<'i>(
input: &mut Parser<'i, '_>,
position: &mut (SourcePosition, TokenSerializationType),
partial_computed_value: &mut ComputedValue,
custom_properties: &CustomPropertiesMap,
Expand Down Expand Up @@ -991,10 +1018,11 @@ fn substitute_block<'i, 't>(
let is_env = name.eq_ignore_ascii_case("env");

partial_computed_value.push(
input,
input.slice(position.0..before_this_token),
position.1,
last_token_type,
);
)?;
input.parse_nested_block(|input| {
// parse_var_function() / parse_env_function() ensure neither .unwrap() will fail.
let name = {
Expand All @@ -1014,7 +1042,7 @@ fn substitute_block<'i, 't>(

if let Some(v) = value {
last_token_type = v.last_token_type;
partial_computed_value.push_variable(v);
partial_computed_value.push_variable(input, v)?;
// Skip over the fallback, as `parse_nested_block` would return `Err`
// if we don't consume all of `input`.
// FIXME: Add a specialized method to cssparser to do this with less work.
Expand All @@ -1036,7 +1064,7 @@ fn substitute_block<'i, 't>(
custom_properties,
env,
)?;
partial_computed_value.push_from(position, input, last_token_type);
partial_computed_value.push_from(input, position, last_token_type)?;
}
Ok(())
})?;
Expand Down Expand Up @@ -1096,6 +1124,6 @@ pub fn substitute<'i>(
&custom_properties,
env,
)?;
substituted.push_from(position, &input, last_token_type);
substituted.push_from(&input, position, last_token_type)?;
Ok(substituted.css)
}

0 comments on commit dca315e

Please sign in to comment.