signalapp / Signal-iOS

A private messenger for iOS.
https://signal.org
GNU Affero General Public License v3.0
10.84k stars 3.04k forks source link

Recieved messages with URLs containing unicode don't display #5861

Closed ethanalker closed 2 months ago

ethanalker commented 2 months ago

Bug description

Sending a link with a unicode character in it causes the message to fail to send. The message shows as delivered on the sender's client, but the reciever sees no message.

Steps to reproduce

Actual result: On the sender's client, the message shows as delivered. On the reciever's client, nothing is shown.

Expected result: The message should appear on the reciever's device.

Additional Notes

Screenshots

Unicode in URL fails to send/recieve Unicode in URL sends to andriod Unicode in URL fails to recieve from andriod

Device info

Device: iPhone 13 & iPhone 12 Pro

iOS version: 16.0 (20A362) & 17.5.1

Signal version: 7.25 (287)

Link to debug log

This debug log contains both sending and receiving a bad link, to/from an iOS device. https://debuglogs.org/ios/7.25.0/c7910952c721686b7f127540a16ae0ac5ecd27dfe9e944bc334bbed5e2047aab.zip

ethanalker commented 2 months ago

Looking through the logs again is looks like root of the issue is here and propogates here (corresponding to the two warnings in the logs). This could be caused by a bad string encoding when creating the string, which seems to be coming from the parseStringEncoding function in HTTPURLResponse, so there may be a bug in that function.

ethanalker commented 2 months ago

I didn't track down what code causes the message not to show, but it may be worth finding where that happens and improving the error handling so the message will show in some capcity even if errors like this occur, or at least make it visible to the user that something happened.

I discovered this when I was sent a link and, from my perspective, I was just left on read. This is a bad user experience and shouldn't happen.

ethanalker commented 2 months ago

This was fixed in 7.26 by breaking embeds with URLs containing unicode. I'd have preferred if it escaped automatically and supported the (technically malformed) URLs, since users still sometimes get their hands on these URLs sometimes from e.g. copying from a browser, but a resolution is a resolution. Closing.