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

Bad regex for detecting SVG on Navigation Links #14334

Closed
omenking opened this issue Jul 26, 2021 · 6 comments · Fixed by #14481
Closed

Bad regex for detecting SVG on Navigation Links #14334

omenking opened this issue Jul 26, 2021 · 6 comments · Fixed by #14481
Assignees
Labels
area: admin admin panel bug smash Approved bugs for the DEV community bug smash bug always open for contribution

Comments

@omenking
Copy link
Contributor

omenking commented Jul 26, 2021

The dreaded error:

Screen Shot 2021-07-26 at 8 57 12 AM

Code in question:

class NavigationLink < ApplicationRecord
  SVG_REGEXP = /<svg .*>/i.freeze

Testing with rubular
Screen Shot 2021-07-26 at 9 03 10 AM

So the problem is that the Regex expects svg text to be all on a single line:

Screen Shot 2021-07-26 at 9 06 04 AM

When exporting svg from illustrator it will sometimes insert a XML schema tag first.

Yep Single line formatting fixed it:

Screen Shot 2021-07-26 at 9 11 39 AM

@github-actions
Copy link
Contributor

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

@rhymes rhymes added area: admin admin panel bug always open for contribution labels Jul 26, 2021
@rhymes rhymes assigned rhymes and unassigned rhymes Jul 26, 2021
@rhymes
Copy link
Contributor

rhymes commented Jul 26, 2021

@omenking I took a quick look but I wasn't able to reproduce, can you please describe what you did and attach the media if possible?

@SiddharthShyniben
Copy link
Contributor

I think this can be fixed by giving the regex the DOTALL (s) flag

@SiddharthShyniben
Copy link
Contributor

@rhymes I think the problem is the current regex doesn't match any newlines, so if the SVG contains newlines in the <svg> tag, this regex doesn't match.

So this matches:

<svg ...>

But this wont:

<svg ...
     ...>

@citizen428
Copy link
Contributor

citizen428 commented Aug 12, 2021

@SiddharthShyniben The flag to make . match newlines in Ruby is m but otherwise, you're correct.

https://rubular.com/r/3CCXiqFPznQdkh

Do you want to make a PR? 😃

@SiddharthShyniben
Copy link
Contributor

@citizen428 Sure! I'll take this up

SiddharthShyniben added a commit to SiddharthShyniben/forem that referenced this issue Aug 12, 2021
citizen428 pushed a commit that referenced this issue Aug 13, 2021
* Fix regex to check svg. Fixes #14334

* Add tests
@cmgorton cmgorton added the bug smash Approved bugs for the DEV community bug smash label Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: admin admin panel bug smash Approved bugs for the DEV community bug smash bug always open for contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants