Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Set style properties directly in mutation #211

Draft
wants to merge 3 commits into
base: sentry-v2
Choose a base branch
from

Conversation

chargome
Copy link
Member

@chargome chargome commented Aug 7, 2024

This PR sets style attributes directly instead of rewriting the full style attribute. That avoids users running into runtime CSP errors when they have not configured unsafe-inline in their CSP.

fixes #145
relates to #10481

@chargome chargome self-assigned this Aug 7, 2024
export function splitStyleAttributes(styles: string) {
const splitStyles = styles.split(';');
return splitStyles
.filter((value) => value.split(':').length == 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Is this safe? Could styles not include colons in other places? Not 100% sure, just want to double check!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will probably be an issue sooner or later, you're right

@chargome chargome marked this pull request as draft August 7, 2024 10:02
@chargome
Copy link
Member Author

I have tried several options on injecting these styles (with nonces), like using a shadow dom, an iframe or just a style tag but all of these were still triggering the csp error.

The only option that really worked were setting the attributes directly using javascript, which is most likely too brittle and expensive to do.

The only other alternative that kinda worked was:

  • creating a style tag on the unattached document
  • setting a nonce on that tag, adding inline styles as a class inside the style tag
  • creating a span, setting the classname we have defined in the style tag
  • using window.getComputedStyle(span)to get the styles and then get the properties from that

but this alters values (e.g. color: red becomes an rgb value), so also not really suitable I think

@visualjerk
Copy link

visualjerk commented Sep 20, 2024

Unsure, if this is viable in your case, but some libraries offer a nonce option on initialization which is added to each injected style element.

Example from emotion

@sminnee
Copy link

sminnee commented Mar 18, 2025

Was there any updates on this @chargome ?

@chargome
Copy link
Member Author

Hey @sminnee no updates tbh, what I wrote in previous comment is still status quo. I will likely not be able to work on this soon but will re-visit at some point – Open for any ideas and contributions meanwhile!

@sminnee
Copy link

sminnee commented Mar 18, 2025

Yeah, it's tricky, the other approach is to avoid needing the temporary createElement at all.

  • Simplify the mutation of the style attribute so that the whole value is marked as a change rather than individual properties.
  • Write a slightly better style attribute parser (e.g. ignore : and ; characters between matched quotes) and directly create the map of propName => [propValue, propPriority] data that is used for comparison below

This is a quick GPT-4o mockup of a parser that handles that case using a regex with lookahead. The regex is a bit of a beast and would need some tests in a real implementation.

function parseStyleString(styleString) {
  const styles = {};

  // Regex to match property-value pairs, accounting for quoted values and !important
  const regex =
    /([\w-]+)\s*:\s*((?:'[^']*'|"[^"]*"|[^;])*?)(\s*!important)?\s*(?=;|$)/g;

  let match;
  while ((match = regex.exec(styleString)) !== null) {
    const property = match[1].trim();
    const value = match[2].trim();
    const priority = match[3] ? "important" : null;

    // Remove quotes from around the value if present
    const unquotedValue = value.replace(/^['"]|['"]$/g, "");
    styles[property] = [unquotedValue, priority];
  }

  return styles;
}

// Example usage:
const styleString =
  'color: red; content: "Hello: World;"; background-color: blue !important; border: 1px red solid';
const parsedStyles = parseStyleString(styleString);
console.log(parsedStyles);

Outputs

{
  color: [ 'red', null ],
  content: [ 'Hello: World;', null ],
  'background-color': [ 'blue', 'important' ],
  border: [ '1px red solid', null ]
}

Do you think this would reduce the brittleness sufficiently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: use of setAttribute('style', ...) in rrweb/src/record/mutation.ts violates CSP style-src directive
4 participants