Bug 34840

Summary:	Doesn't strip newlines from kick/part/… messages
Product:	Telepathy	Reporter:	Will Thompson <will>
Component:	idle	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	Keywords:	patch
Version:	unspecified
Hardware:	Other
OS:	All
URL:	http://cgit.collabora.com/git/user/wjt/telepathy-idle-wjt.git/log/?h=34840-strip-newlines
Whiteboard:
i915 platform:		i915 features:
Attachments:	Connection: replace \r and \n with spaces

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.