Skip to content

[Notifier] [Telegram] Escaping special characters with MarkdownV2 #42697

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.

Sign up for GitHub

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

Jump to bottom
Closed
thomas2411 opened this issue Aug 24, 2021 · 4 comments · Fixed by #42721
Closed

[Notifier] [Telegram] Escaping special characters with MarkdownV2 #42697

thomas2411 opened this issue Aug 24, 2021 · 4 comments · Fixed by #42721
Labels
Bug Notifier Status: Needs Review

Comments

@thomas2411
Copy link
Contributor

Symfony version(s) affected: 5.3.6 and below

Description
When sending notification to Telegram that contains any special characters from the list _ * [ ] ( ) ~` > # + - = | { } . ! , except dot which is already escaped, throws exception:

Message: "Handling "Symfony\Component\Notifier\Message\ChatMessage" failed: Unable to post the Telegram message: Bad Request: can't parse entities: Character '-' is reserved and must be escaped with the preceding '\' (code 400)

It is for default parse_mode or TelegramOptions::PARSE_MODE_MARKDOWN_V2

How to reproduce

Send a notification via telegram with any of the special characters listed above using default parse_mode or TelegramOptions::PARSE_MODE_MARKDOWN_V2

Possible Solution
In the documentation of Telegram Bot API https://core.telegram.org/bots/api#markdownv2-style we can see that we need to escape those characters. I have prepared a Regex for this which we can put here https://github.com/symfony/telegram-notifier/blob/5.3/TelegramTransport.php#L85 , replace str_replace with preg_replace that will escape those characters with \.

If you think this is a good solution please let me know, I will Pull a request.
@thomas2411 thomas2411 added the Bug label Aug 24, 2021
@OskarStark
Copy link
Contributor

Thanks for pointing this out, are you open for a PR and add the regex? It would be great if you can add some tests.

As this is a bugfix it should be applied against 5.3 branch, thank you.

Feel free to ping me in the PR to get a review

@thomas2411
Copy link
Contributor Author

Hello Oskar. Yes, I will prepare a PullRequest. I will try to add a test too.

@thomas2411 thomas2411 mentioned this issue Aug 25, 2021
Merged
fabpot added a commit that referenced this issue Aug 25, 2021
@fabpot
bug #42721 Escape all special characters for parse_mode MARKDOWN_V2 (…
7d4882c 
…thomas2411)

This PR was submitted for the 5.4 branch but it was squashed and merged into the 5.3 branch instead.

Discussion
----------

Escape all special characters for parse_mode MARKDOWN_V2

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #42697
| License       | MIT

In the documentation of Telegram Bot API https://core.telegram.org/bots/api#markdownv2-style we can see that we need to escape more characters _ * [ ] ( ) ~ ` > # + - = | { } . !.
I have prepared a Regex for this and replaced https://github.com/symfony/telegram-notifier/blob/5.3/TelegramTransport.php#L85
I have also changed test used for escaping dot to a test that checks escaping all those characters.

Commits
-------

709981b Escape all special characters for parse_mode MARKDOWN_V2
@OskarStark
Copy link
Contributor

Closed in #42721

@tft7000
Copy link

tft7000 commented Jun 21, 2023

In my understanding, this code change replaces all markdown characters when I choose ->parseMode(TelegramOptions::PARSE_MODE_MARKDOWN_V2).
Doesn't that defeat the purpose of markdown mode?

As far as I understand, currently my only option is, to use the legacy markdown option TelegramOptions::PARSE_MODE_MARKDOWN to send some markdown notification.

Maybe the escaping should only happen, if TelegramOptions::PARSE_MODE_MARKDOWN_V2 is not explicitly chosen in TelegramTransport::doSend.

@faizanakram99 faizanakram99 mentioned this issue Aug 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Notifier Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Escape all special characters for parse_mode MARKDOWN_V2 thomas2411/symfony
4 participants
@thomas2411 @OskarStark @tft7000 @carsonbot