-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
fix: escape the event data for slack webhook payload #18424
Conversation
Codecov Report
@@ Coverage Diff @@
## main #18424 +/- ##
==========================================
- Coverage 67.40% 67.37% -0.04%
==========================================
Files 982 982
Lines 106836 106842 +6
Branches 2670 2670
==========================================
- Hits 72016 71986 -30
- Misses 30950 30983 +33
- Partials 3870 3873 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
{ | ||
name: `escape \\"`, | ||
args: args{str: `{\"foo\":\"bar\"}`}, | ||
want: `{\\\"foo\\\":\\\"bar\\\"}`, | ||
}, |
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.
according to the args
& want
, it seems that the code is escaping \"
, instead of \\"
. Could you please elaborate it? Thanks
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.
If the input is {\"foo\":\"bar\"}
, then escapeEventData
function will firstly escape the symbol "
, so the input now will look like {\\"foo\\":\\"bar\\"}
, but it's not valid because \
also need to be escaped, so escapeEventData
function again convert \\"
=> \\\"
to escape the \
.
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.
sorry, it seems that I understand it now. {\"foo\":\"bar\"}
is replaced to {\\"foo\\":\\"bar\\"}
first by
// escape " to \"
str = strings.Replace(str, `"`, `\"`, -1)
and then replaced to {\\\"foo\\\":\\\"bar\\\"}
by
// escape \\" to \\\"
str = strings.Replace(str, `\\"`, `\\\"`, -1)
77c6313
to
9429d9f
Compare
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.
lgtm
Escape the event data of slack webhook as original payload is invalid when send to slack. Fixes: goharbor#18423 Signed-off-by: chlins <[email protected]>
9429d9f
to
5026beb
Compare
Escape the event data of slack webhook as original payload is invalid when send to slack.
Fixes: #18423
Thank you for contributing to Harbor!
Comprehensive Summary of your change
Issue being fixed
Fixes #18423
Please indicate you've done the following: