392729 - Links for nntp and news Protocols Ignored or crash [@ nsNntpService::MessageURIToMsgHdr(char const*, nsIMsgDBHdr**) ]

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[David E. Ross]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4 Mnenhy/0.7.5.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4

Go to bug #224335.  Scroll down to <https://bugzilla.mozilla.org/show_bug.cgi?id=224335#c9>.  Click on the links to the soc.culture.esperanto and mozilla.general newsgroups.  Nothing happens.  Per RFC 3986, these should be valid links.  

The newsgroups do exist.  The first is part of the Usenet "Big 8".  We all know about the second one.  

Reproducible: Always




I think fixing bug #224335 will depend on this also being fixed.

Comment 1

[Magnus Melin [:mkmelin]]

From that comment, both news:soc.culture.esperanto and nntp://news.mozilla.org/mozilla.general works for me - linux/trunk. Remember to make sure thunderbird is the default news client.

Comment 2

[David E. Ross]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9

Thunderbird version 2.0.0.12 (20080213)

WindowsXP SP2

I selected [Tools > Options > General] on the Thunderbird menu bar.  I then selected the "Check Now" button under System Defaults.  On the Default Client window, I unchecked E-mail (because I use an old version of Eudora) and checked Newsgroups (which was not checked).  I then exited from the Options window by selecting OK buttons.  

With Thunderbird closed, selecting the <nntp://news.mozilla.org/mozilla.general> link (to which I am subscribed) in comment #1 caused Thunderbird to launch; but none of my four newsgroup accounts opened.  With Thunderbird already operating, selecting that link caused a new instance of Thunderbird to launch, again without opening any newsgroup account.  With Thunderbird already operating and with the account for the news.mozilla.org server open, selecting that link caused a new instance of Thunderbird to launch without opening the mozilla.general newsgroup (which is the first in that account's list of newsgroups and which had unread messages).  

With Thunderbird closed, selecting the <news:soc.culture.esperanto> link (to which I am NOT subscribed) in comment #1 caused Thunderbird to launch.  I then saw the effect of bug #41133.  Although I selected Cancel instead of choosing to subscribe, I still a spurious account for a server named news was created.  The same thing happened when I selected the link with Thunderbird operating.  

Launching Thunderbird may be correct when Thunderbird is not already operating.  I question whether it is correct to launch a new instance when Thunderbird is indeed operating.  Further, the nntp link is NOT working since it fails to open the requested newsgroup or even merely open the indicated account.  

Comment 4

[Nikolay Shopik]

Problem still here.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090514 Shredder/3.0b3pre

Comment 5

[Nikolay Shopik]

Clicking on nntp://news.mozilla.org/mozilla.general will crash TB
bp-64faf08b-b460-4744-8fe9-d3d052090717
bp-f9494810-c4a6-495e-8b87-8d7152090717
bp-9c280a59-90fa-4fc3-8093-f25312090717
Raising importance. 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090716 Shredder/3.0b4pre

Comment 6

[Magnus Melin [:mkmelin]]

Crash at http://hg.mozilla.org/comm-central/annotate/eef20451fde6/mailnews/news/src/nsNntpService.cpp#l1841

But i don't crash myself...

Comment 7

[timeless]

Signature	nsNntpService::MessageURIToMsgHdr(char const*, nsIMsgDBHdr**)
UUID	64faf08b-b460-4744-8fe9-d3d052090717
Time 	2009-07-17 03:10:50.60672
Uptime	167
Last Crash	154145 seconds before submission
Product	Thunderbird
Version	3.0b4pre
Build ID	20090716172119
Branch	1.9.1
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 11
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0
User Comments	
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	thunderbird.exe 	nsNntpService::MessageURIToMsgHdr 	mailnews/news/src/nsNntpService.cpp:1841
1 	thunderbird.exe 	nsNntpUrl::GetMessageHeader 	mailnews/news/src/nsNntpUrl.cpp:279
2 	xpcom_core.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101

I can't figure out which path is giving you the null pointer, GetChildNamed seems to have already been spanked/spackled

Comment 8

[Wayne Mery (:wsmwk)]

some are startup crashes like bp-b7aa9c2b-605d-4ce9-8a19-1f20e2091008

bp-9a95e1a4-2e9b-42fc-bee6-4471f2091015 
Right click on newsgroup link, then choose open in browser, Shredder was the default option there. Opened firefox with blamk window

low rank crash ~#100 in 3.0 and 3.0b4

Comment 10

[Ludovic Hirlimann [:Usul]]

(In reply to comment #9)
> *** Bug 536532 has been marked as a duplicate of this bug. ***

STR :
TB crash on callback from "snews://" links


Steps to Reproduce:
1. be sure TB is the OS default newsreader
2. create a page / go to a page with a link like: snews://foo.bar.org
3. view the page on browser and click on the link
Actual Results:  
TB crashes

Comment 11

[Ludovic Hirlimann [:Usul]]

Joshua can you try to fix this ?

Comment 12

[Joshua Cranmer [:jcranmer]]

I've looked into fixing this before, but I thought I found something that wasn't making sense. I'll take a stab at this, but I can't make any guarantees about getting around in a timely manner right now.

Comment 13

[David E. Ross]

Fixing this might require addressing some other bugs.  

I think the relationship between this bug and bug #224335 is obvious.  Fixing one should involve fixing both.  

If none of the news server accounts have subscribed to the requested newsgroup, then what?  The ability of a user to specify a default account (bug #324353) might resolve this issue.  (Bug #324353 might be a duplicate of bug #16343.)  Of course, this issue would surely be resolved by fixing bug #64142.  

Fixing this (or bug #224335) might also resolve bug #4113.  

There might be other bugs to this one.  I will leave it to the experts to update blocking information on these bugs and to resolve whether bug #324353 is a duplicate of bug #16343.

Comment 14

[Joshua Cranmer [:jcranmer]]

Actually, I just noticed:

Has anyone reproduced this bug on a system other than windows?

Comment 15

[Wayne Mery (:wsmwk)]

#158 crash for v3.0.4. might be a regression - there are no crashes prior to 3.0b3 circa 2009071514

my crash bp-f3affdc7-c08b-4f37-be6f-562ea2100414 using https://bugzilla.mozilla.org/show_bug.cgi?id=224335#c9

is it odd that some of these crashes are during startup?  For example 
bp-70ef83df-e75f-4eda-8a0b-8ae312100408
bp-3c6ab53f-1e07-4944-9975-bd3132100413

Comment 16

[Joshua Cranmer [:jcranmer]]

(In reply to comment #15)
> #158 crash for v3.0.4. might be a regression - there are no crashes prior to
> 3.0b3 circa 2009071514

Original reporter was using 2.0 branch; Nikolay also reproduces on 20090514. The C++ code stack also has only one change in hg era, and that was on 20090521.

> is it odd that some of these crashes are during startup?

Not really: the crash is coming from a command-line handler.

Comment 17

[David E. Ross]

I have not seen Shopik's crash.  

I just ran a test from the Web page <http://rossde.com/test/news_nntp.html>, where the use of news: and nntp: reflect F. Ellermann's draft RFC "The 'news' and 'nntp' URI Schemes" at <https://datatracker.ietf.org/doc/draft-ellermann-news-nntp-uri/>.  (Although dated 2 April 2008, this draft has not yet been expired by the IETF and is the latest reference for the news: and nntp: URI schemes.)  See specifically sections 3 and 4 of the draft RFC.  

The URI <nntp://news.mozilla.org/mozilla.support.seamonkey> causes the popup asking if I want to subscribe to mozilla.support.seamonkey.  I am already subscribed to this newsgroup.  I selected Cancel to avoid the problem of generating a spurious account named simply "news".  The result is unaffected by whether or not Thunderbird is running.  A right-click produces a context pull-down menu of nine items.  

The URI <news://mozilla.support.thunderbird/> causes no action, again whether or not Thunderbird is running.  I am also subscribed to this newsgroup.  No, Thunderbird is not launched if it is not already running.  A right-click produces a context pull-down menu of five items.  Ellermann claims the news: URI -- ignored here -- scheme is more widely supported than the nntp: scheme.  

Selecting either URI does not insert any entries into the SeaMonkey Error Console.  

Browser:  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100317 SeaMonkey/2.0.4

News Reader:  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4

Note, however, that I initially observed this problem with SeaMonkey 1 and Thunderbird 2 (or was it 1?).  This is NOT a regression; this is an old problem that has never been fixed.  On the other hand, I recall that this worked correctly with Netscape 4.

Comment 18

[David E. Ross]

Attachment: Pull-Down Context Menu for <news://mozilla.support.thunderbird/>

Comment 19

[David E. Ross]

Attachment: Pull-Down Context Menu for <nntp://news.mozilla.org/mozilla.support.seamonkey>

Comment 20

[David E. Ross]

Note that my test page <http://rossde.com/test/news_nntp.html> also demonstrates the error when copying a news: URI.  This can be seen in my comment #17.  The test page has <news:mozilla.support.thunderbird>, but "Copy Link Location" changes it to <news://mozilla.support.thunderbird/> with // added after news:.  This is a different bug, but I don't know the number right now.

Comment 21

[David E. Ross]

Ellermann's draft RFC is now official.  It's RFC 5538 at <http://www.rfc-editor.org/rfc/rfc5538.txt>.  Mozilla should comply with that RFC.

Comment 22

[Makoto Kato [:m_kato]]

nsNntpService::DecomposeNewsMessageURI returns NS_OK with *aFoloder is null.
When errorCode isn't 0, aFolder keeps null, then return NS_OK.

Should we fix this on 3.1.x timefame?

Comment 23

[Ludovic Hirlimann [:Usul]]

(In reply to comment #22)
> nsNntpService::DecomposeNewsMessageURI returns NS_OK with *aFoloder is null.
> When errorCode isn't 0, aFolder keeps null, then return NS_OK.
> 
> Should we fix this on 3.1.x timefame?

It would be nice - the less crasher the better.

Comment 24

[Makoto Kato [:m_kato]]

(In reply to comment #21)
> Ellermann's draft RFC is now official.  It's RFC 5538 at
> <http://www.rfc-editor.org/rfc/rfc5538.txt>.  Mozilla should comply with that
> RFC.

I file a new bug as bug 601564 to support RFC5538.  So this bug is to fix crash.

Comment 25

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 26

[David :Bienvenu]

Comment on attachment 480584 [details] [diff] [review]
fix

thx for the patch. It would be really good to have some sort of unit test for this since I don't believe there's much test coverage for this area.

Comment 27

[Makoto Kato [:m_kato]]

(In reply to comment #26)
> thx for the patch. It would be really good to have some sort of unit test for
> this since I don't believe there's much test coverage for this area.

I will add unit test.

Comment 28

[Makoto Kato [:m_kato]]

Attachment: unit test

Comment 29

[David :Bienvenu]

Comment on attachment 480871 [details] [diff] [review]
unit test

thx for the patch - I'm going to let jcranmer review it since he said he was working on unit tests (this was said in IRC, unfortunately, not in this bug)...

Comment 30

[David :Bienvenu]

Comment on attachment 480871 [details] [diff] [review]
unit test

In response to Joshua's question on IRC, yes, this test fails (crashes) without the patch.

Comment 31

[Joshua Cranmer [:jcranmer]]

Comment on attachment 480871 [details] [diff] [review]
unit test

>+  var daemon = setupNNTPDaemon();
>+  var localserver = setupLocalServer(NNTP_PORT);
>+  var listener = { OnStopRunningUrl : function() { localserver.closeCachedConnection(); } };
>+
>+  var server = new nsMailServer(new NNTP_RFC977_handler(daemon));
>+  server.start(NNTP_PORT);

You don't need to start up the fakeserver just to see if the URL is valid or not.

>+  try {
>+    // This format isn't supported yet.
>+    // We have to throw a exception until RFC5538 support is landed.
>+    // (Bug 226890)
>+    let hdr = getMessageHeaderFromUrl("nntp://localhost:" + NNTP_PORT +
>+                                      "/test.subscribe.simple");
>+    do_check_true(false); 
>+  } catch (e) {
>+    do_check_true(true);
>+  }

This really shouldn't be here, since it is a bug that we don't support that form of NNTP URIs. Also, group URIs shouldn't have message headers associated with them anyways.

>+  try {
>+    // msgkey is invalid
>+    let hdr = getMessageHeaderFromUrl("news://localhost:" + NNTP_PORT +
>+                                      "/message-id?group=test.subscribe.simple&key=abcdefghijk");
>+    do_check_true(false); 
>+  } catch (e) {
>+    do_check_true(true);
>+  }

Also, please check that the proper error is thrown, instead of just saying that an error is thrown.

In addition, could you also check that this works for a valid message key?

>+  server.stop();
>+
>+  var thread = gThreadManager.currentThread;
>+  while(thread.hasPendingEvents())
>+    thread.processNextEvent(true);
>+}

Similarly, we don't need this part here if we're not using fakeserver.

Comment 32

[Makoto Kato [:m_kato]]

Attachment: unit test v2

Comment 33

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/comm-central/rev/a02c207dff03

Comment 34

[Mark Banner (:standard8)]

Comment on attachment 480584 [details] [diff] [review]
fix

a=Standard8 as long as this lands with the unit test as well.

Comment 35

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/releases/comm-1.9.2/rev/a5d626efa563

Comment 36

[David E. Ross]

The page "Mozilla Thunderbird 3.1.8 Release Notes" at <http://www.mozillamessaging.com/en-US/thunderbird/3.1.8/releasenotes/> cites bugzill.mozill.org query status-thunderbird3.1 as a list of bugs closed in version 3.1.8.  That query includes this bug report.  

However, my testing indicates this bug is still open in 3.1.8.  That is consistent with the Target Milestone of 3.3 for this bug.  

"status-thunderbird3.1: .8-fixed" is incorrect for this bug report and needs to be changed.  That is something I cannot do, so someone else will have to correct that.

Comment 37

[Mark Banner (:standard8)]

(In reply to comment #36)
> However, my testing indicates this bug is still open in 3.1.8.  That is
> consistent with the Target Milestone of 3.3 for this bug.  
>
> "status-thunderbird3.1: .8-fixed" is incorrect for this bug report and needs to
> be changed.  That is something I cannot do, so someone else will have to
> correct that.

Did you actually test 3.3 (your comments are unclear)? I can't reproduce on 3.1.8 or 3.3.

It looks like this bug fixed at least one crash, given the test case. Therefore its possible you are seeing a new, or different crash. Please could you file a separate bug with the STRs and relevant crash ids (and mark it dependent on this one). That way we'll know that this bug did try to fix something in 3.3a2/3.1.8 if we're looking for issues later on.

Comment 38

[Mark Banner (:standard8)]

Hmm, I guess its entirely possible this bug fixed the crash, but not necessarily the ignoring. Still, probably better to have a separate bug for the remaining issue(s).

Comment 39

[Joshua Cranmer [:jcranmer]]

(In reply to comment #38)
> Hmm, I guess its entirely possible this bug fixed the crash, but not
> necessarily the ignoring. Still, probably better to have a separate bug for the
> remaining issue(s).

Getting news URIs generally working is bug 226890.

Comment 40

[David E. Ross]

Hmm.  I originally wrote this bug report.  I was reporting that news: and nntp: protocols do not work.  Somehow, my bug report morphed into something else, something about a crash that I never saw.  

When I tested this yesterday, I still did not see a crash; but I also did not see my original problem solved.  Yes, bug 226890 does address my problem.  I guess the morphed problem of 392729 -- which I never saw -- is indeed fixed.  However, you can see how I became confused.