-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Use old behavior for telegram webhook #31588
Conversation
4c8aefe
to
611c1d1
Compare
611c1d1
to
6e46bbd
Compare
@@ -181,7 +182,7 @@ func (t telegramConvertor) Package(p *api.PackagePayload) (TelegramPayload, erro | |||
|
|||
func createTelegramPayload(message string) TelegramPayload { | |||
return TelegramPayload{ | |||
Message: strings.TrimSpace(message), | |||
Message: markup.Sanitize(strings.TrimSpace(message)), |
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.
Maybe html.EscapeString()
for each fields (author, commit message) is better than santize of final message?
This santizer will not process the commit messages properly... Like that
git commit -am 'Implemented some <a href="https://im.hacker">feature</a>'
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.
That's the old behavior (I do not want to break too much in 1.22)
The complete fix is in Refactor webhook #31587 , it does what you suggested.
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.
Ok, understood! 👍
A more complete fix for #31588 1. Make "generic" code more readable 2. Clarify HTML or Markdown for the payload content
Fix #31182 (only a quick fix, not quite right)