Project

General

Profile

Bug #1173

Quassel adding invalid : on custom server commands

Added by j0s over 7 years ago. Updated almost 6 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
06/12/2012
Due date:
% Done:

100%

Estimated time:
Version:
0.7.3
OS:
Any

Description

I noticed that quassel added an : in front of the last word of a command!
So if I e.g. type "/bs say #chan some text" quassel will send "/BS say #chan some :text"
With a server side alias defined, that ends up as "/msg BotServ say #chan some :text" and a bot will say "some :text" in #chan, while I clearly wanted it to say "some text"
I found the part of the source adding the : andd changed it and I have my quasselcore running with the changes without noticing any problems!
Still, someone added that code with a reason, so please check what it is/was good for!
If there is a valid reason, there needs to be a branch adding the : only if needed and not always!
I sent an pull request on gitorious for this bug!


Related issues

Has duplicate Quassel IRC - Bug #1225: Authserv rejects passwords sent by QuasselClosed2013-05-23

Associated revisions

Revision 155eda45 (diff)
Added by Manuel Nickschas almost 6 years ago

Don't always add a colon to custom commands

The IRC spec mandates that spaces be used as separators between the
arguments for a command. However, the last parameter may itself contain
spaces, and to allow this, one can prefix that argument with a colon.

Instead of checking if the colon is needed, Quassel just always added
a colon to the last argument, which usually works fine. However, it broke
some uses of custom server commands.

This fix now checks if the last argument contains a space or starts with
a colon (which then needs to be escaped), and leaves it alone otherwise.

Fixes #1173 - thanks to Gunnar Beutner for providing the patch.

Revision 39322ca3 (diff)
Added by Manuel Nickschas almost 6 years ago

Don't always add a colon to custom commands

The IRC spec mandates that spaces be used as separators between the
arguments for a command. However, the last parameter may itself contain
spaces, and to allow this, one can prefix that argument with a colon.

Instead of checking if the colon is needed, Quassel just always added
a colon to the last argument, which usually works fine. However, it broke
some uses of custom server commands.

This fix now checks if the last argument contains a space or starts with
a colon (which then needs to be escaped), and leaves it alone otherwise.

Fixes #1173 - thanks to Gunnar Beutner for providing the patch.

History

#1 Updated by Anonymous over 7 years ago

  • Target version set to Some future release

#2 Updated by gunnarbeutner over 6 years ago

Unfortunately I have no idea what the original merge request looked like (i.e. because Gitorious is just giving me "An error has occured. Please try again later." for that URL).

However, if the patch just unconditionally removes all colons this is probably not correct (e.g. for other built-in commands like KICK which use multiple words for the last parameter).

Here's my own patch which only adds colons for the last parameter if it contains spaces. This should be in line with the IRC RFC:

diff --git a/src/core/corenetwork.cpp b/src/core/corenetwork.cpp
index 101f527..c19abc0 100644
--- a/src/core/corenetwork.cpp
+++ b/src/core/corenetwork.cpp
@@ -263,11 +263,14 @@ void CoreNetwork::putCmd(const QString &cmd, const QList<QByteArray> &params, co
         msg += ":" + prefix + " ";
     msg += cmd.toUpper().toAscii();

-    for (int i = 0; i < params.size() - 1; i++) {
-        msg += " " + params[i];
+    for (int i = 0; i < params.size(); i++) {
+        msg += " ";
+
+        if (i == params.size() - 1 && params[i].contains(' '))
+            msg += ":";
+
+        msg += params[i];
     }
-    if (!params.isEmpty())
-        msg += " :" + params.last();

     putRawLine(msg);
 }

#3 Updated by gunnarbeutner over 6 years ago

My patch has a minor problem: It also needs to check if the first character of the last parameter is a colon - and add another colon if it is.

#4 Updated by gunnarbeutner over 6 years ago

Here's a new patch I've been testing for a while now. It works as expected:

diff --git a/src/core/corenetwork.cpp b/src/core/corenetwork.cpp
index 101f527..fc05c64 100644
--- a/src/core/corenetwork.cpp
+++ b/src/core/corenetwork.cpp
@@ -263,11 +263,14 @@ void CoreNetwork::putCmd(const QString &cmd, const QList<QByteArray> &params, co
         msg += ":" + prefix + " ";
     msg += cmd.toUpper().toAscii();

-    for (int i = 0; i < params.size() - 1; i++) {
-        msg += " " + params[i];
+    for (int i = 0; i < params.size(); i++) {
+        msg += " ";
+
+        if (i == params.size() - 1 && (params[i].contains(' ') || (!params[i].isEmpty() && params[i][0] == ':')))
+            msg += ":";
+
+        msg += params[i];
     }
-    if (!params.isEmpty())
-        msg += " :" + params.last();

     putRawLine(msg);
 }

#5 Updated by Anonymous almost 6 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Applied in changeset quassel|commit:155eda45e862f42a0b9444d615002deda461328d.

Also available in: Atom PDF