parseIRCURL doesn't like colons

RESOLVED FIXED

Status

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: torhu, Assigned: Gijs)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cz-0.9.79])

Attachments

(1 attachment)

2.87 KB, patch
bugzilla-mozilla-20000923
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

-

Reproducible: Always

Steps to Reproduce:
1. Open irc://quakenet/TC:Elite.
2. Enable "Open this channel at startup" for that channel.
3. Quit and restart Chatzilla.

Actual Results:  
Error message box:
Invalid IRC URL "irc://quakenet/TC%3aElite".
(Reporter)

Comment 1

12 years ago
Forgot to mention it was Chatzilla 0.9.75.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Autojoin on startup doesn't work for channels with URL-encoded characters → parseIRCURL doesn't like colons

Comment 2

12 years ago
Per the IRC spec itself, colons are not valid in channel names. The URL is, therefore, invalid (in that it points at an invalid resource). We explicitly disallow ^G (0x07), ",", ":" and whitespace within the resource target of the URL because all are invalid per the IRC spec.

We should really disallow issuing a JOIN for such an invalid channel in the first place.

Comment 3

12 years ago
How else would we allow connecting to channels with masks?
<irc://moznet/chatzilla%3a*.mozilla.org> is a valid channel reference, and since it creates a different channel to #chatzilla, it makes sense for a client to treat the mask part as opaque.

Comment 4

12 years ago
Channel masks are not compatible with existing server implementations, as demonstrated by the reporter being in a channel with a colon in the name. They are also not specified in the original IRC spec and completely under-specified in the new IRC specs (2811 in particular). I don't wish to pretend to support them.
(Assignee)

Comment 5

12 years ago
So if we don't want to 'support' masks, that means we should either allow the : normally, so that people like the reporter can still use their channels even in URLs, or we can disallow it in joins too, effectively shutting people out of part of the IRC server, or we can mark this wontfix and keep the status quo. I suppose the decision is with the owner/peers - Rob, Samuel, James, what do you think?

Personally, I feel we should allow it in the URL as well, as disallowing it in joins will shut people out, and the status quo is inconsistent and means we actually *generate* invalid URLs ourselves.
(Reporter)

Comment 6

12 years ago
#TC:Elite gets parsed as a valid link in mIRC, and automatic joining of that channel at startup also works.  Might be a good idea to follow mIRC, since it's probably the most popular client.  And if it doesn't break anything, why not?

Updated

12 years ago
OS: Windows XP → All
Hardware: PC → All

Comment 7

12 years ago
I guess we could allow this and let the server complain when it doesn't support it.

Comment 8

12 years ago
We should ultimately allow : in the channel name, both to allow masks on servers that support it (like Unreal) and the technically invalid names found on Quakenet.

We also need to pick up this error code when attepting to join a channel on a server that supports masks, but with an invalid mask:

[476] #foo:bar Bad Channel Mask
(Assignee)

Comment 9

12 years ago
Created attachment 262864 [details] [diff] [review]
Tested Patch

Sometimes things are surprisingly easy. Let's fix this. I'm open to suggestions on the message, by the way, though I'm quite happy with the current text, perhaps there are better ways of putting it.
Assignee: rginda → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #262864 - Flags: review?(silver)

Updated

12 years ago
Attachment #262864 - Flags: review?(silver) → review+
(Assignee)

Comment 10

12 years ago
Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.241; previous revision: 1.240
done
Checking in mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties;
/cvsroot/mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties,v  <--  chatzilla.properties
new revision: 1.135; previous revision: 1.134
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.79]
You need to log in before you can comment on or make changes to this bug.