Bug 34840 - Doesn't strip newlines from kick/part/… messages
Summary: Doesn't strip newlines from kick/part/… messages
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: idle (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/wj...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-02-28 08:33 UTC by Will Thompson
Modified: 2011-09-12 09:34 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Connection: replace \r and \n with spaces (2.58 KB, patch)
2011-09-09 01:54 UTC, Will Thompson
Details | Splinter Review

Description Will Thompson 2011-02-28 08:33:55 UTC
While reviewing Chandni's fix for bug 34812, I noticed that newlines aren't stripped from the PART message that's built. This means that if you quit #foo as follows:

    RemoveMembers([self_handle], "bye bye\r\nPRIVMSG #telepathy :hai guise")

then the following will be sent on the wire:

    PART #foo :bye bye
    PRIVMSG #telepathy :hai guise

rather than the intended:

    PART #foo :bye bye PRIVMSG #telepathy :hai guise

That is: the second half of the message will be interpreted by the server as a separate command. This is a contrived example, of course.

The same bug afflicts kick messages. A quick grep suggests there might be a few other places with this bug.
Comment 1 Will Thompson 2011-09-09 01:54:25 UTC
Created attachment 50995 [details] [review]
Connection: replace \r and \n with spaces

If a message gets as far as _send_with_priority and contains \r or \n,
it's almost certainly not what the user anticipated. For instance,
before this fix, calling

    RemoveMembers([self_handle], "bye\r\nJOIN #telepathy")

would cause the user to leave the channel with the message "bye", and
then accidentally join #telepathy.

Rather than trying to strip out \r and \n everywhere, this patch just
replaces them with spaces just before sending.
Comment 2 Will Thompson 2011-09-09 03:51:22 UTC
Comment on attachment 50995 [details] [review]
Connection: replace \r and \n with spaces

Hmm, this depends on another branch of mine.
Comment 3 Will Thompson 2011-09-09 03:52:13 UTC
See the referenced branch. It, and the branch on bug 40734, both depend on 'testier-better-faster-stronger' which cleans up the tests and makes them orders of magnitude faster via a dirty hack.
Comment 4 Debarshi Ray 2011-09-10 02:21:31 UTC
The patch looks good to me.

I only wanted to make sure that if the user copy pasted a huge chunk of text, then the newlines were being taken care of before reaching _send_with_priority. Now I see that idle_text_send calls idle_text_encode_and_split to do this, so I am happy. :-)
Comment 5 Will Thompson 2011-09-12 09:34:05 UTC
Thanks!

I split apart the commit in testier-better-faster-stronger you asked to be split, and merged both branches.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.