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 Rehydration regex causing infinite loops for malformed inputs #3017

Closed
kitten opened this issue Feb 12, 2020 · 0 comments · Fixed by #3018
Closed

Fix Rehydration regex causing infinite loops for malformed inputs #3017

kitten opened this issue Feb 12, 2020 · 0 comments · Fixed by #3018
Labels

Comments

@kitten
Copy link
Member

kitten commented Feb 12, 2020

The regex based rehydration can fall into an infinite loop in some edge cases. As part of this issue we should:

  • Update the regex to prevent the edge case in the first place
  • Investigate why it caused an infinite loop and protect against similar loops to prevent this issue in the future

Edit: I'm now actually not convinced it's an infinite loop. I've got a minimal reproduction that eventually completes after a very long time, so it may be a Regex catastrophic backtracking issue.

This issue was discovered in #2972. There some CSS was being output (malformed by stylis due to /*! comments) and caused the following rule to be input into rehydration:

{/*! normalize.css v8.0.1 | MIT License | github.com/necolas/normalize.css *//*!                                                                                                                               
 *  Font Awesome 4.7.0 by @davegandy - http://fontawesome.io - @fontawesome                                                                                                                                    
 *  License - http://fontawesome.io/license (Font: SIL OFL 1.1, CSS: MIT License)                                                                                                                              
 */} 

This causes an infinite loop since (presumably) it's a selector-less rule body.

Edit: Because I forgot as well, to recap why we switched away from CSSOM-based rehydration in #2872: There are quirks and bugs in browsers that cause cssText on rules to be unreliable 🤦‍♂ See: https://bugzilla.mozilla.org/show_bug.cgi?id=1594241

Edit 2: This test here gets slower as more characters (one by one!) are added to the invalid normalize.css comment line:

  it.only('tolerates malformed CSS', () => {
    document.head.innerHTML = `
      <style ${SC_ATTR} ${SC_ATTR_VERSION}="${SC_VERSION}">
        {/*! normalize.css v8.
          }
        .rule {}
        ${SC_ATTR}.g1[id="idA"]{content:""}
      </style>
    `;

    const sheet = new StyleSheet({ isServer: true });
    rehydrateSheet(sheet);
  });

Edit 3: Content doesn't matter so it seems to somehow be the newline in the rule, which usually is stripped out by stylis, so this may be why it's such a rare edge case (?)

Edit 4: Using /([^{]*){((?:{[^}]*}|(?!{).*?)*)}/g; instead (removing .*) and adding trim later helps to keep the time down but after a couple more characters the time grows again. So there's another backtracking issue in here.

kitten added a commit that referenced this issue Feb 12, 2020
quantizor added a commit that referenced this issue Mar 5, 2020
* Fix RULE_RE to reduce backtracking

Fix #3017

* Add CHANGELOG entry

* revise SSR emitter & rehydrator to use a split sequence instead of regex

* Fix splitting to be per rule

* Update CHANGELOG entry

* Add newline to SPLITTER constant

* skip garbage nodes

* adjust snapshot

Co-authored-by: Evan Jacobs <probablyup@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant