Bug #1173
Quassel adding invalid : on custom server commands
100%
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
Associated revisions
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
#2 Updated by Anonymous over 11 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> ¶ms, 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 Anonymous over 11 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 Anonymous over 11 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> ¶ms, 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 over 10 years ago
- Status changed from New to Resolved
- % Done changed from 0 to 100
Applied in changeset quassel|commit:155eda45e862f42a0b9444d615002deda461328d.
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.