search

python-discord / bot

The community bot for the Python Discord community
https://pythondiscord.com
MIT License
1.37k stars 675 forks source link

discord timestamps used in invalid embed fields #1751

Closed Numerlor closed 2 years ago

Numerlor commented 3 years ago

The discord Markdown timestamps introduced in #1666 are invalid in some embed fields like footers where they will render as plaintext leading ton on human readable data being sent. The watchchannel send_header method tries to use a timestamp in the footer https://github.com/python-discord/bot/blob/4d62dadc4d19f0c17d5cda8748c3fc520f2adccf/bot/exts/moderation/watchchannels/_watchchannel.py#L285-L296 And a reminder that's late tries to use a timestamp as a name in the author field https://github.com/python-discord/bot/blob/4d62dadc4d19f0c17d5cda8748c3fc520f2adccf/bot/exts/utils/reminders.py#L191-L194

MarkKoz commented 3 years ago

There's a similar issue with this log message

https://github.com/python-discord/bot/blob/ad163c41e097c95bda033007b7422b129cdbbcf1/bot/exts/moderation/stream.py#L140-L141

From this and my experience with #1721, it's become evident that #1666 was not thoroughly reviewed and tested. It's a bit concerning that several obvious errors were missed, but that might just be me having the power of hindsight. I think it's worth reviewing all uses of these Discord timestamp utility functions to make sure they're only used where they render properly.

D0rs4n commented 3 years ago

I'd like to take this issue, if it's up for grabs. (As a matter of fact, this issue has been already patched in case of the reminders)

TizzySaurus commented 2 years ago

From searching for around 30 minutes, the only issue that remains today that I could find was the log statement in Mark's above comment, and in discussion with @D0rs4n we decided it's not really worth PRing a fix since it has humanised time with it anyway.

As such, this issue can likely be closed, but @D0rs4n wishes to take a look themselves before we do so.

D0rs4n commented 2 years ago

Yeah, I went through all the possible places where this could go wrong, and I could only find that log statement as well. I observed this log statement more, and I believe it would be great if we could refactor this log message, though. Just for the sake of consistency. So I'll keep this issue open, it should be a fairly easy patch. (The mentioned log message again: https://github.com/python-discord/bot/blob/9bae829f91c00f89fd1e725036f4d24ec796410b/bot/exts/moderation/stream.py#L137 ) @TizzySaurus

TizzySaurus commented 2 years ago

@D0rs4n done in #2100 :+1: