Closed Bug 954813 Opened 10 years ago Closed 10 years ago

Avoid sending empty nicks when changing modes with tab complete

Categories

(Chat Core :: IRC, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1378 at 2012-04-13 21:20:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1378 as attmnt 1352 at 2012-04-13 21:20:00 UTC ***

When you tab complete a mode change you end up with something like "/op clokep " (note the space at the end). Currently we try to send two nicks: "clokep" and "". We should trim this string first to avoid the second message.
Attachment #8353105 - Flags: review?(florian)
Comment on attachment 8353105 [details] [diff] [review]
Patch

*** Original change on bio 1378 attmnt 1352 at 2012-04-16 10:08:21 UTC ***

"a  b".split(" ").toSource()
->
["a", "", "b"]

I suspect you want a .filter(function(n) n) instead of the trim.

What happens if the user finds some way (with a copy/paste maybe?) to insert other invalid characters (tab, unbreakable space, ...)?
Attachment #8353105 - Flags: review?(florian) → review-
*** Original post on bio 1378 at 2012-04-16 10:19:52 UTC ***

(In reply to comment #1)
> Comment on attachment 8353105 [details] [diff] [review] (bio-attmnt 1352) [details]
> Patch
> 
> "a  b".split(" ").toSource()
> ->
> ["a", "", "b"]
> 
> I suspect you want a .filter(function(n) n) instead of the trim.
> 
> What happens if the user finds some way (with a copy/paste maybe?) to insert
> other invalid characters (tab, unbreakable space, ...)?

Have I mentioned I hate sanitizing user input?

It probably needs to be something like aMsg.split("\s").filter(function(n) n.length). I'll see what I can come up with though. Thanks for finding this flaw!
Assignee: nobody → clokep
Attached patch Patch v2 (obsolete) — Splinter Review
*** Original post on bio 1378 as attmnt 1369 at 2012-04-18 22:33:00 UTC ***

This should be more robust to the situations flo suggested.

I applied it to all the places we need to split user input.
Attachment #8353122 - Flags: review?(florian)
Comment on attachment 8353105 [details] [diff] [review]
Patch

*** Original change on bio 1378 attmnt 1352 at 2012-04-18 22:33:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353105 - Attachment is obsolete: true
Attached patch Patch v2.1Splinter Review
*** Original post on bio 1378 as attmnt 1370 at 2012-04-18 23:02:00 UTC ***

I had posted the wrong patch it seems. :(
Attachment #8353123 - Flags: review?(florian)
Comment on attachment 8353122 [details] [diff] [review]
Patch v2

*** Original change on bio 1378 attmnt 1369 at 2012-04-18 23:02:29 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353122 - Attachment is obsolete: true
Attachment #8353122 - Flags: review?(florian)
Comment on attachment 8353123 [details] [diff] [review]
Patch v2.1

*** Original change on bio 1378 attmnt 1370 at 2012-04-18 23:19:14 UTC ***

Nice one :)
Attachment #8353123 - Flags: review?(florian) → review+
Severity: trivial → minor
Whiteboard: [checkin-needed]
*** Original post on bio 1378 at 2012-04-23 23:02:32 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/ec52c0464439
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
Whiteboard: [checkin-needed]
You need to log in before you can comment on or make changes to this bug.