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

feat: adds includePattern option #63

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@morgs32
Copy link

commented Jun 5, 2019

Closes #62

@ljharb
Copy link
Member

left a comment

See #62 (comment); this seems like something your babel config can do without any individual transform needing to participate.

@@ -1,3 +1,6 @@
# IDE configs
.idea

This comment has been minimized.

Copy link
@ljharb

ljharb Jun 5, 2019

Member

individual IDE configs should go in your global gitignore, not in every project you happen to touch.

This comment has been minimized.

Copy link
@morgs32

morgs32 Jun 6, 2019

Author

Best advice I've gotten all day.

if (ignorePattern) {
if (includePattern) {
// Only set the includeRegex once:
includeRegex = includeRegex || new RegExp(includePattern);

This comment has been minimized.

Copy link
@ljharb

ljharb Jun 5, 2019

Member

it's dangerous to pass user input into RegExp; this is a DOS attack vector, for example.

This comment has been minimized.

Copy link
@morgs32

morgs32 Jun 17, 2019

Author

@ljharb In case you come around to the idea of adding this option, might you suggest how I could resolve this issue? I've copied the behavior of the ignorePattern almost exactly: https://github.com/airbnb/babel-plugin-inline-react-svg/pull/63/files#diff-1fdf421c05c1140f6d71444ea2b27638R65

This comment has been minimized.

Copy link
@ljharb

ljharb Jun 17, 2019

Member

Instead of supporting regex, it should only support globs (gitignore syntax) - you can use https://npmjs.com/glob for that, i think

This comment has been minimized.

Copy link
@morgs32

morgs32 Jun 17, 2019

Author

Ok cool I'll give that a shot

This comment has been minimized.

Copy link
@morgs32

morgs32 Jun 17, 2019

Author

Actually I've been trying to find some literature about this vulnerability - this is the best I can do so far: https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS.

It doesn't seem to me that this is a DOS risk (not like that at least). Is there precedent for protecting the engineer from doing this to his/herself? I suppose another babel preset could end up doing this - but still I'd have to have deliberately installed a malicious preset, right? Thoughts?

@morgs32 morgs32 closed this Jun 6, 2019

@ljharb ljharb reopened this Jun 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.