Skip to content

fix: avoid escaping unnecessary chars in HTML report suppression regexes#6749

Merged
jeremylong merged 1 commit intodependency-check:mainfrom
chadlwilson:stop-escaping-hyphens
Jun 30, 2024
Merged

fix: avoid escaping unnecessary chars in HTML report suppression regexes#6749
jeremylong merged 1 commit intodependency-check:mainfrom
chadlwilson:stop-escaping-hyphens

Conversation

@chadlwilson
Copy link
Copy Markdown
Collaborator

@chadlwilson chadlwilson commented Jun 30, 2024

Description of Change

Bit of a nitpick: when the HTML report generates an example suppression, it unnecessarily escapes hyphens when constructing a regex inside HTML.

e.g. pkg:maven/org.eclipse.jetty.websocket/websocket-jetty-server

becomes

   <packageUrl regex="true">^pkg:maven/org\.eclipse\.jetty\.websocket/websocket\-jetty\-server@.*$</packageUrl>

This is a little more annoying to read and maintain than it needs to be :-) This would be better:

   <packageUrl regex="true">^pkg:maven/org\.eclipse\.jetty\.websocket/websocket-jetty-server@.*$</packageUrl>

Within regexes, - only needs to be escaped within a character class, and only if not the first character in the character class (ironically the old code relied on this). Since we are not putting elements into character classes and the the relevant [ is escaped we don't need to escape this, and it reads better to not escape.

Similar

  • # has no special meaning in regex, not sure why it was being escaped
  • , has no special meaning outside a repetition qualifier (already {} are escaped), so similarly does not need to be escaped

Technically the escaping is a bit janky right now as it is also escaping some things for the purpose of using JavaScript to construct XML so technically it needs to escapeForXml afterwards, or the packageUrl = pkg:hello-world-</packageUrl> will get you into broken XML trouble, but I guess that is not a major concern :-)

I guess escaping the whitespace escaping with \s is one of these XML-related workarounds based on #2046 so have left it around.

Have test cases been added to cover the new functionality?

no

@boring-cyborg boring-cyborg Bot added the core changes to core label Jun 30, 2024
@chadlwilson chadlwilson force-pushed the stop-escaping-hyphens branch from aa16a77 to 156adb5 Compare June 30, 2024 08:11
Copy link
Copy Markdown
Collaborator

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

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

lgtm

@jeremylong jeremylong merged commit 9b42aed into dependency-check:main Jun 30, 2024
@chadlwilson chadlwilson deleted the stop-escaping-hyphens branch June 30, 2024 09:36
@jeremylong jeremylong added this to the 10.0 milestone Jun 30, 2024
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

core changes to core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants