-
Notifications
You must be signed in to change notification settings - Fork 210
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
ui: Fix escaped characters bug #2198
Conversation
setLabelValues(response.labelValues); | ||
// replace single `\` in the `labelValues` string with doubles `\\` if available. | ||
const newValues = response.labelValues.map(value => | ||
value.includes('\\') ? value.replace('\\', '\\\\') : value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I belive the string.replace
will only replace the first matching pattern, so it wouldn't handle a second occurrence of \\
in the string.
I am not sure if multiple escape characters is a possibility here, but just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, Should we extract this into a sanitizeLabelValue
function in @parca/functions
package and use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I just figured that was a possibility. making a change now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great that the CodeQL scanner captured it too. 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!! Works perfectly now. lgtm 👍 🎉
is there a docker image / release I could use with this fix? |
Yep! There's always an image built from |
* ui: Fix escaped characters bug * Use `replaceAll` to switch characters * Fix linting errors Co-authored-by: Manoj Vivek <p.manoj.vivek@gmail.com>
This PR fixes a bug where an inclusion of escaped characters in the
parca.yaml
file causes the profiler not to work as expected. To fix, we replace single\
in thelabelValues
string with doubles\\
if available and this is done on 3 levels:Resolves #1060