Last Comment Bug 842024 - (euirc) Thunderbird Chat can't connect to EuIRC
(euirc)
: Thunderbird Chat can't connect to EuIRC
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: 17 Branch
: All All
: -- normal (vote)
: Thunderbird 21.0
Assigned To: Patrick Cloke [:clokep]
:
Mentors:
Depends on: 842183
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-16 05:52 PST by mozillapersona
Modified: 2013-02-18 07:05 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to make \r optional (3.06 KB, patch)
2013-02-16 10:29 PST, Patrick Cloke [:clokep]
aleth: review+
Details | Diff | Splinter Review
Followup patch v1 (4.52 KB, patch)
2013-02-17 07:47 PST, Patrick Cloke [:clokep]
aleth: review-
Details | Diff | Splinter Review
Followup patch v2 (4.72 KB, patch)
2013-02-17 07:56 PST, Patrick Cloke [:clokep]
aleth: review+
Details | Diff | Splinter Review
Alt followup patch, ver 1 (2.42 KB, patch)
2013-02-17 10:35 PST, Patrick Cloke [:clokep]
florian: review+
Details | Diff | Splinter Review

Description mozillapersona 2013-02-16 05:52:12 PST
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
Comment 1 Florian Quèze [:florian] [:flo] 2013-02-16 06:36:17 PST
What's the error message displayed before it attempts to connect again? Are there errors in the Error Console?
Comment 2 mozillapersona 2013-02-16 06:41:34 PST
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
Comment 3 Patrick Cloke [:clokep] 2013-02-16 08:18:03 PST
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.
Comment 4 Patrick Cloke [:clokep] 2013-02-16 10:12:28 PST
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
Comment 5 Patrick Cloke [:clokep] 2013-02-16 10:24:26 PST
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...
Comment 6 Patrick Cloke [:clokep] 2013-02-16 10:29:46 PST
Created attachment 714808 [details] [diff] [review]
Patch to make \r 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.
Comment 7 mozillapersona 2013-02-16 11:35:12 PST
(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 8 aleth [:aleth] 2013-02-16 12:06:58 PST
Comment on attachment 714808 [details] [diff] [review]
Patch to make \r optional

Thanks for diagnosing this so quickly!
Comment 9 Patrick Cloke [:clokep] 2013-02-16 21:48:01 PST
(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.
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-02-17 04:19:09 PST
This is bitrotted. Please rebase and attach a new patch.
Comment 11 Florian Quèze [:florian] [:flo] 2013-02-17 04:59:59 PST
(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 :).
Comment 12 Patrick Cloke [:clokep] 2013-02-17 07:47:35 PST
Created attachment 714907 [details] [diff] [review]
Followup patch v1

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?
Comment 13 aleth [:aleth] 2013-02-17 07:50:01 PST
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.
Comment 14 Patrick Cloke [:clokep] 2013-02-17 07:56:07 PST
Created attachment 714908 [details] [diff] [review]
Followup patch v2

And this is why I wasn't very happy about having to hack around this...
Comment 15 aleth [:aleth] 2013-02-17 07:58:16 PST
Comment on attachment 714908 [details] [diff] [review]
Followup patch v2

The things we do because servers don't follow the specs... ;)
Comment 16 mozillapersona 2013-02-17 08:09:21 PST
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 :-)
Comment 17 Patrick Cloke [:clokep] 2013-02-17 08:54:28 PST
(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!
Comment 18 Patrick Cloke [:clokep] 2013-02-17 10:35:40 PST
Created attachment 714941 [details] [diff] [review]
Alt followup patch, ver 1

Florian seems to prefer this, I tested it on moznet, freenode and euirc.
Comment 19 Patrick Cloke [:clokep] 2013-02-17 14:40:16 PST
This will be ported from Instantbird in bug 842183.
Comment 20 Patrick Cloke [:clokep] 2013-02-17 16:15:41 PST
Attachment 714941 [details] [diff] committed to Instantbird as http://hg.instantbird.org/instantbird/rev/3cfc081fb133

Note You need to log in before you can comment on or make changes to this bug.