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

Fatal Error in REGEX Scape fixed in blacklist.js #458

Closed

Conversation

RohitChattopadhyay
Copy link

Summary
Error: In Windows 10
Invalid regular expression: /(node_modules[\\\]react[\\\]dist[\\\].*|website\\node_modules\\.*|heapCapture\\bundle\.js)$/: Unterminated character class

Modification:
In the first index of sharedBlacklist variable, the regex write was without scape, it is a path with a purpose to exclude react/dist of metro process.
The problem is resolved by escaping.
Reference Issue: #453

Test plan
The fix resolved this issue and Metro started working after the modifications.
Tested using command react-native start

Thanks :)

Error:
`Invalid regular expression: /(node_modules[\\\]react[\\\]dist[\\\].*|website\\node_modules\\.*|heapCapture\\bundle\.js)$/: Unterminated character class`
Modification fix:
In the first index of sharedBlacklist variable, the regex write was without scape, it is a path with a purpose to exclude react/dist of metro process.
The problem is resolved by this edit.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 9, 2019
@codecov-io
Copy link

Codecov Report

Merging #458 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #458   +/-   ##
=======================================
  Coverage   84.43%   84.43%           
=======================================
  Files         173      173           
  Lines        5764     5764           
  Branches      949      949           
=======================================
  Hits         4867     4867           
  Misses        794      794           
  Partials      103      103
Impacted Files Coverage Δ
packages/metro-config/src/defaults/blacklist.js 60% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48dddff...80b0329. Read the comment docs.

@RohitChattopadhyay RohitChattopadhyay changed the title Falat Error in REGEX Scape fixed in blacklist.js Fatal Error in REGEX Scape fixed in blacklist.js Oct 10, 2019
Copy link

@igrir igrir left a comment

Choose a reason for hiding this comment

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

I tested this on my windows machine and it worked

@cpojer
Copy link
Contributor

cpojer commented Oct 29, 2019

Thank you for your PR! Fixed in #468.

@cpojer cpojer closed this Oct 29, 2019
facebook-github-bot pushed a commit that referenced this pull request Oct 29, 2019
Summary:
**Summary**

On Windows with Node.js v12.x.x, Metro crashes with
```
SyntaxError: Invalid regular expression: /(.*\\__fixtures__\\.*|node_modules[\\\]react[\\\]dist[\\\].*|website\\node_modules\\.*|heapCapture\\bundle\.js|.*\\__tests__\\.*)$/: Unterminated character class
```
This has been reported in #453, facebook/react-native#26829, facebook/react-native#26969, facebook/react-native#26878, facebook/react-native#26598, expo/expo-cli#1147 and expo/expo-cli#1074.

There are a few open pull requests attempting to fix this same issue:
* #464
* #461
* #458
* #454

However, none of the existing PRs address the *root cause* of this error: the `escapeRegExp` function in `blacklist.js` tries to convert regular expressions to be agnostic to the path separator ("/" or "\\"), but turns some valid regular expressions to invalid syntax.

The error was is this line:
https://github.com/facebook/metro/blob/142348f5345e40ce2075fc7f9dfa30c5d31fee2a/packages/metro-config/src/defaults/blacklist.js#L28
When given a regular expression, such as `/node_modules[/\\]react[/\\]dist[/\\].*/`, on Windows where `path.sep` is `\` (which is also an escape character in regular expressions), this gets turned into `/node_modules[\\\]react[\\\]dist[\\\].*/`, resulting in the `Unterminated character class` error.

Automatically replacing `[/]` with `[\]` is an error, as is replacing `[\/]` with `[\\]`, because in both of these cases the backslash before the end of character class "]" escapes it, and the character class becomes unterminated. Therefore, this PR changes the code to look for both escaped forward slash `\/` and forward slash `/`, and always replace them with the escaped version (`\/` or `\\`, depending on the platform).

This fixes #453.

**Test plan**

Added a test case that exercises the code with both `\` and `/` as path separators.
Pull Request resolved: #468

Differential Revision: D18201730

Pulled By: cpojer

fbshipit-source-id: 6bb694178314c39d4d6a0fd9f8547bfa2c36f894
cpojer pushed a commit that referenced this pull request Jan 8, 2020
Summary:
**Summary**

On Windows with Node.js v12.x.x, Metro crashes with
```
SyntaxError: Invalid regular expression: /(.*\\__fixtures__\\.*|node_modules[\\\]react[\\\]dist[\\\].*|website\\node_modules\\.*|heapCapture\\bundle\.js|.*\\__tests__\\.*)$/: Unterminated character class
```
This has been reported in #453, facebook/react-native#26829, facebook/react-native#26969, facebook/react-native#26878, facebook/react-native#26598, expo/expo-cli#1147 and expo/expo-cli#1074.

There are a few open pull requests attempting to fix this same issue:
* #464
* #461
* #458
* #454

However, none of the existing PRs address the *root cause* of this error: the `escapeRegExp` function in `blacklist.js` tries to convert regular expressions to be agnostic to the path separator ("/" or "\\"), but turns some valid regular expressions to invalid syntax.

The error was is this line:
https://github.com/facebook/metro/blob/142348f5345e40ce2075fc7f9dfa30c5d31fee2a/packages/metro-config/src/defaults/blacklist.js#L28
When given a regular expression, such as `/node_modules[/\\]react[/\\]dist[/\\].*/`, on Windows where `path.sep` is `\` (which is also an escape character in regular expressions), this gets turned into `/node_modules[\\\]react[\\\]dist[\\\].*/`, resulting in the `Unterminated character class` error.

Automatically replacing `[/]` with `[\]` is an error, as is replacing `[\/]` with `[\\]`, because in both of these cases the backslash before the end of character class "]" escapes it, and the character class becomes unterminated. Therefore, this PR changes the code to look for both escaped forward slash `\/` and forward slash `/`, and always replace them with the escaped version (`\/` or `\\`, depending on the platform).

This fixes #453.

**Test plan**

Added a test case that exercises the code with both `\` and `/` as path separators.
Pull Request resolved: #468

Differential Revision: D18201730

Pulled By: cpojer

fbshipit-source-id: 6bb694178314c39d4d6a0fd9f8547bfa2c36f894
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants