547423 - Cancel messages should send content-type

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[Mike Lykov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9.1.8) Gecko/20100216 Thunderbird/3.0.2

1. I wrote message in usenet newsgroup - for example, test group kraft.test.
2. I want to cancel this _my_ message from server.

When I want to cancel message in usenet newsgroup and press "del", confirm deleting, and get usual dialog with "message deleted".

BUT when I press OK, I got next message: "NNTP Error: Non text/plain Content-Type is not supported in this group, sorry.."
AND message is not deleted really.
I can see it when I choose group properties -> rebuild index.

This server configured to prohibit non text/plain messages in all groups but kraft.binary.
In kraft.binary I CAN create and cancel message.

In Outlook Express I CAN cancel messages in kraft.test (it deletes from server and do not appear in thunderbird) 

Why thunderbird send strande content-type when cancel message in "text/plain only" group?



Reproducible: Always

Steps to Reproduce:
1. send message to group
2. try to cancel it
3. get error "Non text/plain Content-Type is not supported in this group"

Comment 1

[Joshua Cranmer [:jcranmer]]

The exact cancel message posted is thus:

From: [eliding email address]
Newsgroups: mozilla.test

Subject: cancel <[e-mail]>

References: <[e-mail]>

Control: cancel <[e-mail]>



This message was cancelled from within Mozilla Thunderbird.



This is a valid cancel message per RFC 1036 and RFC 5537, and it does have a text/plain content-type, as the default per RFC 2045 is text/plain.

Comment 2

[Ludovic Hirlimann [:Usul]]

(In reply to comment #1)

> This message was cancelled from within Mozilla Thunderbird.
> 
> 
> 
> This is a valid cancel message per RFC 1036 and RFC 5537, and it does have a
> text/plain content-type, as the default per RFC 2045 is text/plain.

So are you saying that the bug is server side and this is an INVALID bug entry Joshua ?

Comment 3

[Mike Lykov]

> Newsgroups: mozilla.test

why mozilla.test and not kraft.test? newsserver news.kraft-s.ru is open and you can try and verify this issue.

> This message was cancelled from within Mozilla Thunderbird.

maybe group on your server is not resticted to text/plain?
 
> This is a valid cancel message per RFC 1036 and RFC 5537, and it does have a
> text/plain content-type, as the default per RFC 2045 is text/plain.

but why the _client_ is showing that message? I can attach a screenshot, and you can test on your own (client and that server). 

Why client show ok "message cancelled" when it get error from server and than show this error?

I think this bug is easy verifyable, then it's not invalid.

Comment 4

[Joshua Cranmer [:jcranmer]]

(In reply to comment #3)
> > Newsgroups: mozilla.test
> 
> why mozilla.test and not kraft.test? newsserver news.kraft-s.ru is open and you
> can try and verify this issue.

I was demonstrating the format of a cancel message we send.

Judging from the INN source code, it doesn't bother to add a default Content-Type: text/plain header for the article filtering scripts, and the filtering script in place on your news server doesn't appear to handle the case of the empty Content-Type header.

> I think this bug is easy verifyable, then it's not invalid.

INVALID basically refers to "is it actually a bug in Thunderbird". I need to go investigate news cancelling some more to ascertain that, though.

Comment 5

[Mike Lykov]

I look at cancel message from Knode:

------------------------
Control: cancel <hm3mdp$tq3$[e-mail]>
Newsgroups: kraft.test
Lines: 1
From: Mike Lykov <[e-mail]>
Subject: cancel of <hm3mdp$tq3$[e-mail]>
Date: Wed, 24 Feb 2010 21:08:44 +0400
Organization: HH
User-Agent: KNode/4.3.4
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit

cancel by original author
------------------

news.kraft-s.ru server Administrator recognized than there is a bug in his server-side filtering scripts and Mozilla thunderbird "has a right" to send his current format of cancel message without content-type.

But Knode or Outlook Express send message like as above with not-required lines.

There is a 2 way to do with all it:

1. Make a workaround againist buggy server-side filtering scripts and careless administrators. This is not required by RFC. but allow thunderbird to work as others on bad configured servers.
Then close this issue.

2. Close issue without any changes wirh recommendation to server administrators to straight their  hands and to clients to not use Thunderbird or go to other server.

You can choose any ;)

Comment 6

[Joshua Cranmer [:jcranmer]]

The other client I looked at, tin, didn't send a Content-Type header.

The original concern I had with implementing this was that one of the new RFC specs for usenet messages mentioned other content types for control messages. On a closer read, those appear to come into play only for moderators and group control.

This would also be an excellent chance to clean up some of the cancel code.

Comment 7

[Nagarjuna Varma [:Infinity]]

Hey! I would like to work on this bug if no one has taken it up yet! Should I discuss a little more on the bug with jcranmer before trying something out?

Comment 8

[Joshua Cranmer [:jcranmer]]

Thank you for volunteering to fix this bug.

What effectively needs to be done in this bug is to add a Content-Type header to the cancel message that is generated here: <http://mxr.mozilla.org/comm-central/source/mailnews/news/src/nsNNTPProtocol.cpp#3747>, although I would also like to see some cancel tests added to our automated test suites.

Comment 9

[Evan Hart [:wline]]

I'd like to do this bug if :infinity isn't already working on it, jcranmer can you confirm if it's ok to work on it?

Comment 10

[Joshua Cranmer [:jcranmer]]

(In reply to Evan Hart:wline from comment #9)
> I'd like to do this bug if :infinity isn't already working on it, jcranmer
> can you confirm if it's ok to work on it?

Yes, it would be okay.

Comment 11

[surabhi anand]

jcranmer: I would like to work on this bug ,but need some more steps on how to approach the issue.

BY far-

1- I am not sure if i have understood correct but we have to implement content type header to recognize differnt kind of contents like text ,pictures etc ?

2- We have to implement standard cancellation message ?

3- Is the issue really in thunderbird or on server side,i mean should i be only concerned about writing content type ?

Please correct me and guide me through.

Comment 12

[Joshua Cranmer [:jcranmer]]

(In reply to surabhi anand from comment #11)
> 1- I am not sure if i have understood correct but we have to implement
> content type header to recognize differnt kind of contents like text
> ,pictures etc ?
> 
> 2- We have to implement standard cancellation message ?
> 
> 3- Is the issue really in thunderbird or on server side,i mean should i be
> only concerned about writing content type ?

The problem stems from a broken filter script on the server side that is attempting to only allow text/plain messages (as mentioned in comment #1, no Content-Type is equivalent to a declaration of a text/plain message). That said, there appears to be no downside towards adding the Content-Type header from within Thunderbird which would help the cancel message get through to the broken server.

Comment 13

[:aceman]

(In reply to Joshua Cranmer [:jcranmer] from comment #12)
> 1- I am not sure if i have understood correct but we have to implement
> content type header to recognize differnt kind of contents like text
> ,pictures etc ?
>
> The problem stems from a broken filter script on the server side that is
> attempting to only allow text/plain messages (as mentioned in comment #1, no
> Content-Type is equivalent to a declaration of a text/plain message).

So the content-type specifies the format of the Cancellation-message (which is always text/plain) ?
Not the type of the original message being cancelled?

Comment 14

[Joshua Cranmer [:jcranmer]]

(In reply to :aceman from comment #13)
> So the content-type specifies the format of the Cancellation-message (which
> is always text/plain) ?
> Not the type of the original message being cancelled?

That is indeed correct.

Comment 15

[:aceman]

Thanks, so now it should be known what needs to be done here. You also describe it in comment 8.

And it seems nobody is working on it right now.

Comment 16

[rsachira]

I would like to take on this bug if no one is working on it. But I will need some help as this is my first bug

Comment 17

[:aceman]

Hi, you should find help in the previous comments. There is even exact link to code block which needs to be touched.

Comment 18

[rsachira]

I added the content-type field to otherHeaders as:
  otherHeaders.AppendLiteral("Content-Type: text/plain");
  otherHeaders.AppendLiteral(CRLF);

I can't figure out how to get the message string generated from DoCancel from tests written in js.

Comment 19

[rsachira]

I know how to make an nntp fakeserver and read new messages but I can't figure out how to send a cancel message to it. Would appreciate if someone can provide help on this. Thanks

Comment 20

[:aceman]

Thanks, the mentor of this bug should be able to tell you what kind of tests he would like here.

Comment 21

[rsachira]

I checked sending a message to news.kraft-s.ru -> k-s.test2 It works without an error. Please let me know what type of test needed.

Comment 22

[rsachira]

Attachment: mychanges.patch

Content type "text/plain" added. Does not include any unit tests.

Comment 23

[Joshua Cranmer [:jcranmer]]

(In reply to rsachira from comment #18)
> I can't figure out how to get the message string generated from DoCancel
> from tests written in js.

Are you referring to the output value of the message? When using the fakeserver in tests, the results of a post are added to the nntpDaemon as any other article is, which means you should be able to query for the last message in the appropriate group to get it. For example:

// test code....
let folder = localserver.rootFolder.getChildNamed("test.filter");
// ...
folder.cancelMessage(hdr, dummyMsgWindow);

let article = daemon.getGroup("test.filter")[daemon.getGroupStats("test.filter")[2]];
// Now you have the full article text of the message!

(You can use test_internalUris.js's test_cancelMessage function to get a lot more context about actually driving this part).

Comment 24

[Alfred Peters [:infofrommozilla]]

(In reply to rsachira from comment #22)
> Created attachment 8859218 [details] [diff] [review]
> mychanges.patch
> 
>  Content type "text/plain" added. Does not include any unit tests.

'Content-Type' is defined by RFC 2045. To give 'Content-Type' a meaning, RFC 2045 requires the 'MIME-Version' header.

Therefore it would be in my opinion a good idea to also set that header:

MIME-Version: 1.0

Comment 25

[grvi.osource]

Hi, is this bug still exists, if so i would like to work on it,

Comment 26

[Alfred Peters [:infofrommozilla]]

(In reply to rsachira from comment #22)
> Created attachment 8859218 [details] [diff] [review]
> mychanges.patch
> 
>  Content type "text/plain" added. Does not include any unit tests.

Are you still working on completing the patch?
If not, please release the Assignee status (Assignee -> [e-mail]).

Comment 27

[Alfred Peters [:infofrommozilla]]

(In reply to Alfred Peters from comment #24)

> Therefore it would be in my opinion a good idea to also set that header:
> 
> MIME-Version: 1.0

@Joshua Cranmer: Would you accept a patch that sets this header?

Comment 28

[Alfred Peters [:infofrommozilla]]

(In reply to Alfred Peters from comment #26)

Comment 29

[Joshua Cranmer [:jcranmer]]

(In reply to Alfred Peters from comment #27)
> (In reply to Alfred Peters from comment #24)
> 
> > Therefore it would be in my opinion a good idea to also set that header:
> > 
> > MIME-Version: 1.0
> 
> @Joshua Cranmer: Would you accept a patch that sets this header?

Yes. The missing piece in the patch is the tests.

Comment 30

[Alfred Peters [:infofrommozilla]]

Attachment: Add Content-Type and Mime-Version header to the NNTP CancelMessage.

Comment 31

[Alfred Peters [:infofrommozilla]]

(In reply to Alfred Peters from comment #30)
> Created attachment 8973465 [details] [diff] [review]
> Add Content-Type and Mime-Version header  to the NNTP CancelMessage.

I just ran the test on the Daily. He should actually work on the beta, release or SeaMonkey. Or do we have to check that first?

Comment 32

[Jorg K (CEST = GMT+2)]

Comment on attachment 8859218 [details] [diff] [review]
mychanges.patch

Superseded by Alfred's patch.

Comment 33

[Jorg K (CEST = GMT+2)]

Attachment: Bug547423_v2.patch

I simplified the test a bit. This is ready for landing now.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d569f238fd31
Add Content-Type and Mime-Version header to the NNTP CancelMessage. r=jorgk