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

checkNodes function has incorrect regex #15

Open
TotallyInformation opened this issue Jan 31, 2022 · 4 comments
Open

checkNodes function has incorrect regex #15

TotallyInformation opened this issue Jan 31, 2022 · 4 comments

Comments

@TotallyInformation
Copy link

TotallyInformation commented Jan 31, 2022

checknodes.js - node-red/node-red-dev-cli - GitHub1s

/<script.+?type=['"]text\/javascript['"].*?>([\S\s]*?)<\/script>/ig

This does not match valid HTML v5 javascript script tag which may not have the type attribute set.

From MDN:

The HTML5 specification urges authors to omit the attribute rather than provide a redundant MIME type.

This results in:

    ---Validating Nodes---
Unable to parse  nodes/wiser.js
Unable to parse  nodes/wiser-listen.js

It would probably be better to have a negative check to eliminate script tags that have type="text/html" or the older red attributes.

@sammachin
Copy link
Collaborator

@hardillb I think this is something you could look at in your node parsing library

@hardillb
Copy link
Member

hardillb commented Feb 4, 2022

Try this:

/<script.?(?:type=['"]text\/javascript['"])?\s?>([\S\s]*?)<\/script>/ig

It probably needs a little more work, but that should match both

<script type="text/javascript">

And

<script>

@Alkarex
Copy link
Contributor

Alkarex commented Feb 4, 2022

A regex will never be robust for such cases. A proper DOM parsing + XPath or CSS query would be better

@knolleary
Copy link
Member

It is largely a question of return on investment. The regex has been good enough for a number of years. A small tweak will fix it for this new case. Replacing the code with a full DOM parsing approach is certainly doable, but will require a lot of changes to the existing code rather than one strategic tweak to the regex.

Longer term there is a plan to pull out the node parsing code into a shared library that can be reused by this tool, the flow-library and Node-RED itself - each of which have a slightly different set of requirements on what information gets pulled out. That would be the place to consider an alternative approach to the parsing.

hardillb added a commit to hardillb/node-red-module-parser that referenced this issue Feb 7, 2022
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

No branches or pull requests

5 participants