RITlug / teleirc

Go implementation of a Telegram <=> IRC bridge for use with any IRC channel and Telegram group
https://docs.teleirc.com/
GNU General Public License v3.0
138 stars 46 forks source link

Escape more non-message strings as plaintext (e.g. action messages, join/part messages, etc.) #157

Closed Tjzabel closed 3 years ago

Tjzabel commented 5 years ago

When using Matrix to talk over IRC, Matrix-created links (from uploaded files for instance) don't show up on Telegram.

Tjzabel commented 5 years ago
* ctmartin uploaded an image: IMG_20190731_125214.jpg (83KB) <https://matrix.org/_matrix/media/v1/download/matrix.org/DjGRGkcJxzqDymJRobhLZXIu>

Here is an example of a matrix image being sent to IRC. My initial feeling is Telegram may be viewing the message as a command, similar to the /me command.

jwflory commented 5 years ago

@Tjzabel Hmm. So the message does not show up completely? I am shooting in the dark here, but the Matrix file uploads seem to use the emote message format (i.e. /me says hi!). I noticed the URL is also wrapped in angle brackets… could it be that emote messages are not being escaped as plaintext and they are accidentally being parsed as HTML?

xforever1313 commented 5 years ago

Here's the raw IRC message we get from the socket (taken from xbot's logs):

:ctmartin!ctmartinma@rit/foss/student/ctmartin PRIVMSG #rit-foss :[0x0001]ACTION uploaded an image: IMG_20190731_125214.jpg (83KB) < https://matrix.org/_matrix/media/v1/download/matrix.org/DjGRGkcJxzqDymJRobhLZXIu >[0x0001]

@Tjzabel - your suspicion is right, it is an action just like /me.

[0x0001] is a control character for "Start of Heading"

Tjzabel commented 5 years ago

@xforever1313 huh, that's odd. Thanks for confirming :+1:

Tjzabel commented 5 years ago

I wonder if we should file a ticket to have Matrix change how they do that, or if we should catch that action.

Just seems to be a very odd mechanism for sending images over.

jwflory commented 5 years ago

@xforever1313 Do you have the raw IRC message of a normal action message to compare against the Matrix PRIVMSG message? It is not clear to me how catching a normal action message (which works today) is different from catching this action message from Matrix.

@Tjzabel I think it requires a lot of persuasion to convince upstream to change this behavior. And even then, it could be a full year, or longer, before we would see this changed if they decided to accept the RFE.

Tjzabel commented 5 years ago

@jwflory what does RFE mean?

But yes, I agree it would be a longshot to do so. I'm just not entirely sure what the reasoning was for doing it this way.

jwflory commented 5 years ago

Sorry, RFE == Request For Enhancement!

xforever1313 commented 5 years ago

I do... here's one that DID go over the bridge:

:dzho!~dzho@unaffiliated/dzho PRIVMSG #rit-foss :[0x0001]ACTION nods[0x0001]

Compared to the one that did not:

:ctmartin!ctmartinma@rit/foss/student/ctmartin PRIVMSG #rit-foss :[0x0001]ACTION uploaded an image: IMG_20190731_125214.jpg (83KB) < https://matrix.org/_matrix/media/v1/download/matrix.org/DjGRGkcJxzqDymJRobhLZXIu >[0x0001]

I got nothing... they look the same to me other than the message that did not go through has more special characters.

jwflory commented 5 years ago

@xforever1313 Thanks! This confirmed my suspicion. :smile: So, action messages are not being escaped as plaintext. They need the same fix that @Tjzabel did for regular message types.

In #ritlug-teleirc, I sent two action messages:

* jwf test
* jwf tests <surprise>

The first message made it to Telegram but the second did not. So I think this issue means we need to actually escape more strings as plaintext.

Tjzabel commented 5 years ago

@jwflory great catch!! This'll be a fun one to do then :smile:

Tjzabel commented 5 years ago

Additional background:

Unhandled rejection Error: ETELEGRAM: 400 Bad Request: can't parse entities: Unsupported start tag "" at byte offset 63
    at request.then.resp (/home/tim/Code/teleirc/node_modules/node-telegram-bot-api/src/telegram.js:280:15)
    at tryCatcher (/home/tim/Code/teleirc/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/tim/Code/teleirc/node_modules/bluebird/js/release/promise.js:517:31)
    at Promise._settlePromise (/home/tim/Code/teleirc/node_modules/bluebird/js/release/promise.js:574:18)
    at Promise._settlePromise0 (/home/tim/Code/teleirc/node_modules/bluebird/js/release/promise.js:619:10)
    at Promise._settlePromises (/home/tim/Code/teleirc/node_modules/bluebird/js/release/promise.js:699:18)
    at _drainQueueStep (/home/tim/Code/teleirc/node_modules/bluebird/js/release/async.js:138:12)
    at _drainQueue (/home/tim/Code/teleirc/node_modules/bluebird/js/release/async.js:131:9)
    at Async._drainQueues (/home/tim/Code/teleirc/node_modules/bluebird/js/release/async.js:147:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/home/tim/Code/teleirc/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

@jwflory it looks like it may not just be an escaping issue, but only more testing will tell.

jwflory commented 3 years ago

I think this was leftover from the v1 -> v2 port. ACTION messages do appear in Telegram now, even if they do look awkward. I am going to close this issue as complete. I removed it from the project board and milestone tracker.