Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Notifier] [Discord] Use private const and mb_strlen()#39492
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
nicolas-grekas commentedDec 14, 2020
Can you please check how Discord behaves as I wondered in my comment ? |
OskarStark commentedDec 14, 2020
| { | ||
| protectedconstHOST ='discord.com'; | ||
| privateconstMAX_SUBJECT_LENGTH =2000; |
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 called TEXT_LIMIT in the Slack bridge 5.2
maybe SUBJECT_LIMIT here? or another name in the Slack bridge?
| $content =$message->getSubject(); | ||
| if (\strlen($content) >2000) { | ||
| if (mb_strlen($content) >self::MAX_SUBJECT_LENGTH) { |
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.
calling mb_* without an explicit charset is something that should be banned in all situations.
It's a bit like gambling with charsets :)UTF-8 must be specified here (and only utf8 is supported, since we use json along the path to Discord)
connorhu commentedDec 17, 2020
It seems ok. Should I run some tests with discord api? |
src/Symfony/Component/Notifier/Bridge/Discord/DiscordTransport.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Notifier/Bridge/Discord/DiscordTransport.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
fabpot commentedDec 17, 2020
Thank you@OskarStark. |
like proposed by@nicolas-grekas inhttps://github.com/symfony/symfony/pull/39444/files#r542288432