Closed Bug 842024 (euirc) Opened 11 years ago Closed 11 years ago

Thunderbird Chat can't connect to EuIRC

Categories

(Thunderbird :: Instant Messaging, defect)

17 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 21.0

People

(Reporter: mozillapersona, Assigned: clokep)

References

Details

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130201065344

Steps to reproduce:

I tried to connect to irc.euirc.net using my registered username and password. Then I tried different servers, listet here: http://www.euirc.net/en/server_admins.php


Actual results:

Connection retries over and over - but no connection.


Expected results:

Connection established, join password protected channels? I don't use any other IRC server than euirc.net
Alias: ssvx
Alias: ssvx → euirc
What's the error message displayed before it attempts to connect again? Are there errors in the Error Console?
Ah, had to realize there's something like an error console. Sorray! Here it is:

Timestamp: 16.02.2013 15:39:36
Error: Connection closed by server.
Source File: resource:///components/irc.js
Line: 494
Source Code:
irc

Timestamp: 16.02.2013 15:40:25
Error: Connection closed by server.
Source File: resource:///components/irc.js
Line: 494
Source Code:
irc

Timestamp: 16.02.2013 15:41:01
Error: Connection closed by server.
Source File: resource:///components/irc.js
Line: 494
Source Code:
irc
I've also been unable to connect with Instantbird, which has more up to date IRC code than Thunderbird. Do you know of a client that DOES work? I'd like to compare the data being sent.
I found the issue with this, euirc only sends a \n as a message terminator, not \r\n, as specified in RFC 1459 [1] and 2812 [2]. I'm trying to contact them to see what's up with that now.

(I was able to get Instantbird to connect fine by changing the message terminator, but I don't really like this solution...)

[1] http://tools.ietf.org/html/rfc1459#section-2.3.1
[2] http://tools.ietf.org/html/rfc2812#section-2.3.1
Status: UNCONFIRMED → NEW
Ever confirmed: true
1:06:59 PM - The topic for #euirc is: Welcome to euIRC || Problems? #hilfe / #help || www.euirc.net - board.euirc.net || IPv6 irc6.euirc.net || Webchat http://webchat.euirc.net || http://nopaste.euirc.net.
1:07:01 PM - Channel mode +tnrCcWf set by irc.inn.at.euirc.net.
1:08:36 PM - clokep_test: I've got a couple of questions about the ircd running on euirc (I think it's a custom one), someone reported a bug that a client I wrote doesn't support connecting...and I had to hack it a bit to get it to work. Would this be the right place to talk about that?
1:09:08 PM - mensch: hi, yes
1:10:02 PM - clokep_test: Hey, thanks. So it seems like euirc only puts a \n at the end of messages and not a \r\n as the RFCs state, what's up with that?
1:13:23 PM - mensch: yes, euircd is a custom one. some networks decided to use \n for whatever reason and in this case euirc is one of them. your client should be fine treating \r as optional.
1:13:52 PM - clokep_test: We don't treat \r as optional. I've never run across another network that does this.
1:14:18 PM - clokep_test: Seems kind of silly to arbitrarily choose not to send \r also, when it's very clearly specified as required.
1:14:27 PM - alamar: clokep_test: it's legacy from 13 years ago - as all common clients deal with it perfectly fine (except yours apparently) there never was a reason to change it
1:15:05 PM - clokep_test: Alright.
1:15:27 PM - clokep_test: Every other ircd I've connected to does it properly though.
1:15:54 PM - alamar: well as there are only 3-4 common implementations left that's not toooo surprising ;) 
1:15:55 PM - mensch: also, many networks that existed 13 years ago aren't there anymore. so euirc is probably one of the few still using
1:16:28 PM - alamar: but you will be pleased that there actually are plans to fix that in the near future ;)
1:16:37 PM - clokep_test: :) Excellent.
1:16:47 PM - mensch: as we aren't that happy with it too
1:16:53 PM - alamar: s/pleased/& to hear/
1:16:56 PM - clokep_test: Is there a bug tracker or something to CC myself to? I couldn't find any info on the euircd.
1:17:12 PM - alamar: no public one, sorry ;)
1:17:44 PM - alamar: so anyway - saturday evening, off to drink ;) 
1:17:49 PM - clokep_test: Can I ask approximately what "the near future" is? (I know that's a dangerous question when releasing software!)
1:17:53 PM - clokep_test: Thanks for the help.
1:18:19 PM - alamar: clokep_test: I'd say this year *duck* ;) more accurately first or second quarter 
1:18:20 PM - mensch: near future as in "we will be alive when it's happening"
1:18:34 PM - alamar: (or maybe third or fourth *duck again)
1:18:40 PM - clokep_test: Those sound like the responses we give people, I should have expected that. :_D
1:18:59 PM - mensch: keep the ducking for your drinking alamar
1:19:03 PM - alamar: it's done when it's done and it only took duke nukem forever like 12 years ;) 
1:19:44 PM - clokep_test: Fair enough. I have to check on the schedule then and see when the next release of Thunderbird is then...

Patch coming up to make it optional...
This makes the \r optional, which I'm not super thrilled about, but \r is usually not allowed in any IRC messages, so this should be OK.

This also fixes some spacing and removes an unused parameter from ircMessage.
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #714808 - Flags: review?(aleth)
(In reply to Patrick Cloke [:clokep] from comment #5)
> Patch coming up to make it optional...

That sounds great!

> This makes the \r optional, which I'm not super thrilled about, but \r is usually not allowed in any IRC messages, so this should be OK.

It might comfort you to think about it as Windows/Linux compatibility hack. I'm a php programmer by the way - and as almost every developer (who has to handle "the end of lines" in character streams) I ran into the \r?\n issue quite often, because UNIX/Linux generally just uses \n for "new line" and Windows uses \r\n for "return carriage new line".

You might remember typewriters or c64 and DOS text editors, where there was no automatic "return carriage" when you entered a "new line" with cursor down. That's why our big [return] key has a "down then left arrow" - it stands for "return carriage (go to char 1) and new line (one line down)". As alamar stated it's a "legacy of old times" - a relic - and while Windows sticks with it, UNIX/Linux never saw the need for it.

Hope this comforts you. At least it does so for me - when interpreting CSV or any other text files or server streams that could come from either source like  Windows or Linux machine, we all need to treat \r as optional.

It's sad that it's not exactly as RFC states, but if I'm not mistaken, that RFC is almost as old as this DOS/UNIX issue :-)

Thanks so much for so quickly taking the time to look into it! It's amazing and I'd never had expected this ambitiousness! Thank you! :-)
Comment on attachment 714808 [details] [diff] [review]
Patch to make \r optional

Thanks for diagnosing this so quickly!
Attachment #714808 - Flags: review?(aleth) → review+
(In reply to mozillapersona from comment #7)
> It might comfort you to think about it as Windows/Linux compatibility hack.
> I'm a php programmer by the way - and as almost every developer (who has to
> handle "the end of lines" in character streams) I ran into the \r?\n issue
> quite often, because UNIX/Linux generally just uses \n for "new line" and
> Windows uses \r\n for "return carriage new line".
I quite understand the platform differences. This has nothing to do with platform line endings though. The protocol is speced out and should be followed appropriately by both clients and servers. The only reason I'm OK with adding this hack AT ALL is the idea of being liberal in what you accept.

> It's sad that it's not exactly as RFC states, but if I'm not mistaken, that
> RFC is almost as old as this DOS/UNIX issue :-)
The RFC is from 1993, euIRC is from 2000, they have no excuse to not implement it properly.

> Thanks so much for so quickly taking the time to look into it! It's amazing
> and I'd never had expected this ambitiousness! Thank you! :-)
You're welcome! It should be in the next version of Thunderbird released. It seems that euIRC does some other funky things that I came across when connecting; please file more bugs if there are other issues.
Keywords: checkin-needed
This is bitrotted. Please rebase and attach a new patch.
Status: ASSIGNED → NEW
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 21.0
(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> This is bitrotted. Please rebase and attach a new patch.

This patch was created against the chat/ folder of the Instantbird repository and applies cleanly there (I just checked it in as http://hg.instantbird.org/instantbird/rev/4c298be422b5). So unfortunately it's not the patch here that's outdated, but comm-central. A quick diff of the chat folders of the 2 repositories shows "45 files changed, 754 insertions(+), 392 deletions(-)". I'll try to update comm-central soon.

Thanks for attempting to check this patch in anyway :).
Attached patch Followup patch v1 (obsolete) — Splinter Review
This was (unfortunately) checked-in for Instantbird and broke all non-euIRC networks. Guess I missed this in one of my tests.

This patch won't apply for Thunderbird right now, but I'm unsure of Florian's plans of syncing c-c and ib, will this patch happen as part of that? Is that happening today / soon?
Attachment #714907 - Flags: review?(aleth)
Comment on attachment 714907 [details] [diff] [review]
Followup patch v1

Sorry to have missed that delimiter is also used for sending in review!

Please change the comment above the delimiter definitions to make it clear why there is a difference, i.e. that inDelimiter can be a regex.
Attachment #714907 - Flags: review?(aleth) → review-
Attached patch Followup patch v2 (obsolete) — Splinter Review
And this is why I wasn't very happy about having to hack around this...
Attachment #714907 - Attachment is obsolete: true
Attachment #714908 - Flags: review?(aleth)
Comment on attachment 714908 [details] [diff] [review]
Followup patch v2

The things we do because servers don't follow the specs... ;)
Attachment #714908 - Flags: review?(aleth) → review+
Sorry for causing so much trouble.

> You're welcome! It should be in the next version of 
> Thunderbird released. It seems that euIRC does some 
> other funky things that I came across when connecting; 
> please file more bugs if there are other issues.

Other "funky stuff" for euIRC are custom name prefixes for channel admin and channel owner. I think they use *founder and !admin but there's a custom user mode +c to make the server use @ for @operator as compatibility prefix for them. More info about that funny business: http://www.euirc.net/en/patches.php

To decide if it's something that should be integrated is up to you - I imagine it's pretty nasty running around and including everyones "customs". So far I'm glad you're making it compatible to connect :-)
(In reply to mozillapersona from comment #16)
> Sorry for causing so much trouble.
> 
> > You're welcome! It should be in the next version of 
> > Thunderbird released. It seems that euIRC does some 
> > other funky things that I came across when connecting; 
> > please file more bugs if there are other issues.
> 
> Other "funky stuff" for euIRC are custom name prefixes for channel admin and
> channel owner. I think they use *founder and !admin but there's a custom
> user mode +c to make the server use @ for @operator as compatibility prefix
> for them. More info about that funny business:
> http://www.euirc.net/en/patches.php
Yeah, I saw that. Mind filing a new bug?

> To decide if it's something that should be integrated is up to you - I
> imagine it's pretty nasty running around and including everyones "customs".
> So far I'm glad you're making it compatible to connect :-)
Luckily it's very easy to separate most of this stuff in our code. Generally I'm OK with including stuff...just not always interested in writing the patches. Connection compatibility is a must though!
Florian seems to prefer this, I tested it on moznet, freenode and euirc.
Attachment #714941 - Flags: review?(florian)
Attachment #714941 - Flags: review?(florian) → review+
This will be ported from Instantbird in bug 842183.
Status: NEW → ASSIGNED
Depends on: 842183
Attachment #714908 - Attachment is obsolete: true
https://hg.mozilla.org/comm-central/rev/87e8a0c36afe
https://hg.mozilla.org/comm-central/rev/dad38b5c0099
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.