170727 - A point [dot] "." at the beginning of a news post line will sometimes be doubled

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[Frank Widmaier]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2a) Gecko/20020907 MultiZilla/v1.1.22
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2a) Gecko/20020907 MultiZilla/v1.1.22

In <amne7r$798u4$[e-mail]> wrote someone:
| Content-Type: text/plain; charset=ISO-8859-1
| Content-Transfer-Encoding: 8Bit

<quote>
body {font-size:medium;}
.a {font-size:10px;}
.b {font-size:10pt;}
.c {font-size:10ex;}
</quote>

some people answered to that message and got back the following:

<quote>
> body {font-size:medium;}
> ..a {font-size:10px;}
> ..b {font-size:10pt;}
</quote>

their preferences:
Message-ID: <ams3p8$8ka6d$[e-mail]>
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2b) Gecko/20020923
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
=> this poster said he will see it in the original text, in the edit-window and
after loading it from the newsserver
Message-ID: <amqa89$5ngbs$[e-mail]>
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.2a) Gecko/20020911
Hamster/1.3.23.4
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8Bit

I couldn't reproduce
Message-ID: <ams3nj$8iak5$[e-mail]>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2a)
Gecko/20020907 MultiZilla/v1.1.22 Hamster/1.3.23.191
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

also couldn't
Message-ID: <amsd8j$48f$02$[e-mail]>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.2a) Gecko/20020910
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 8bit

We imagine, that it could be perhaps the "." at the beginning of the line. One
"." represents the End of a posting in NNTP. So you habe to code the "." that
it's representing the char "."
Perhaps mozilla won't take away the second "." when fetching the message from
the news-server?

Reproducible: Sometimes

Steps to Reproduce:
wo can't reproduce it - it happens

Comment 1

[Karsten Düsterloh]

This bug is still in the 1.3b release!
I've found a way to reproduce it (but I don't have confirm rights):

According to RFC977, lines in news postings starting with the period "." are
transmitted with this dot doubled "..". The newsreader has to take care of that
and decode it back to ".".

If a posting is shown directly from the server, the display is correct. Lines
starting with a period show only one period.
(This happens if news are read *online* and the group is *not* selected for
download, i.e. read postings are *not* stored locally. It occurs also if
displaying the posting's source, because this still :( requires to be in online
mode.)

If a group is selected for download, displayed postings will be stored locally. 
The first time a posting is shown, it is displayed as if no storing takes place
- one dot at line start (obviously, the posting shown comes directly from the
server).
*Every* display of this posting afterwards uses the *locally* *stored* posting -
and this fails. Postings shown from local storage show *two* periods at line start.

This bug does *not* with mail, only with news.

Comment 2

[Karsten Düsterloh]

One additional remark:
Since this "double point" is part of the nntp protocol and not part of the
posting, this bug should be fixed at the point of *storing* the message locally.
The display routines should not cope with that.

Comment 3

[Mike Cowperthwaite]

*** Bug 349524 has been marked as a duplicate of this bug. ***

Comment 4

[(not reading, please use [e-mail] instead)]

sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.

Comment 5

[Joshua Cranmer [:jcranmer]]

Simple fix: nsNNTPProtocol::DisplayArticle doesn't handle the double dot properly, while nsNNTPProtocol::ReadArticle does.

I'll tackle this once I get bug 413077 in so I can hopefully write a testcase for this problem.

Comment 6

[:aceman]

Joshua, bug 413077 is fixed, will you tackle this?

Comment 7

[Thomas 'PointedEars' Lahn]

Bug still there in Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120713 Thunderbird/14.0, according to <news:jvgabm$md7$[e-mail]>.

According to <news:jvkaec$42q$[e-mail]>, it only happens with offline reading.

Comment 8

[Magnus Melin [:mkmelin]]

I suppose somewhere around here: http://hg.mozilla.org/comm-central/annotate/9637efa949a4/mailnews/news/src/nsNNTPProtocol.cpp#l2144

Comment 9

[prateeksurana3255]

Is this issue still active

Comment 10

[Alfred Peters [:infofrommozilla]]

Attachment: Remove the escaped dot from body lines before saving in the offline store.

I don't see any reason why the double dot was saved here in the first place.
If TB saves the articles directly offline, he also removes the dot.

Comment 11

[Alfred Peters [:infofrommozilla]]

Attachment: Remove the escaped dot from body lines before saving in the offline store. bug170727.patch

I added a test.
For that I had to trick a little. Apparently the dot handling in the test server does not work correctly either. But the result is in line with expectations. With patch, the test delivers 100% PASS. Without the patch, the second test fails:

PASS run_test - [run_test : 40] "From: \\"Demo User\\" <[e-mail]>\\nMessage-ID: <[e-mail]>\\nNewsgroups: dot.test\\nSubject: Bug 170727: Test article with dots\\n\\n. 1 dot\\n.. 2 dots\\n"
                             == "From: \\"Demo User\\" <[e-mail]>\\nMessage-ID: <[e-mail]>\\nNewsgroups: dot.test\\nSubject: Bug 170727: Test article with dots\\n\\n. 1 dot\\n.. 2 dots\\n"
FAIL run_test - [run_test : 56] "From: \\"Demo User\\" <[e-mail]>\\nMessage-ID: <[e-mail]>\\nNewsgroups: dot.test\\nSubject: Bug 170727: Test article with dots\\n\\n.. 1 dot\\n... 2 dots\\n"
                             == "From: \\"Demo User\\" <[e-mail]>\\nMessage-ID: <[e-mail]>\\nNewsgroups: dot.test\\nSubject: Bug 170727: Test article with dots\\n\\n. 1 dot\\n.. 2 dots\\n"

Comment 12

[Alfred Peters [:infofrommozilla]]

Attachment: Remove the escaped dot from body lines before saving in the offline store.

(In reply to Alfred Peters from comment #11)

For that I had to trick a little. Apparently the dot handling in the test server does not work correctly either.

When Bug 1669277 will be fixed, the test works without the /tricks/ ;-)

Comment 13

[Alfred Peters [:infofrommozilla]]

(In reply to Alfred Peters from comment #12)

Created attachment 9179655 [details] [diff] [review]

review? Joshua Cranmer

I took him because he was suggested by Buzilla and he has written this code once. But he is absent since august.
The other suggestion was 'neil' but he had already 22 reviews in queue.

So would anybody else do the review of this and bug 1669277?

Comment 14

[Ben Campbell]

Comment on attachment 9179655 [details] [diff] [review]
Remove the escaped dot from body lines before saving in the offline store.

Review of attachment 9179655 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!

::: mailnews/news/src/nsNNTPProtocol.cpp
@@ -2013,5 @@
>      } else  // we aren't finished with the message yet
>      {
>        uint32_t count = 0;
>  
>        // skip over the quoted '.'

I'd probably add a comment with the term "dot-stuffing" (or a link to https://tools.ietf.org/html/rfc3977#section-3.1.1, say) for the benefit of future archeologists.

::: mailnews/news/test/unit/postings/post3.eml
@@ +3,5 @@
> +Newsgroups: dot.test
> +Subject: Bug 170727: Test article with dots
> +
> +. 1 dot
> +.. 2 dots

I'd probably slip in a few more nasty cases to give it a good workout, eg:
=====
. 1 dot
.. 2 dots
     .indented dot
.
no - that wasn't the end of the message!
=====

Comment 15

[Alfred Peters [:infofrommozilla]]

Attachment: Remove the escaped dot from body lines before saving in the offline store.

(In reply to Ben Campbell from comment #14)

I'd probably add a comment with the term "dot-stuffing" (or a link to
https://tools.ietf.org/html/rfc3977#section-3.1.1, say) for the benefit of
future archeologists.
Done.

I'd probably slip in a few more nasty cases to give it a good workout, eg:
Done.

Comment 16

[Magnus Melin [:mkmelin]]

Please remember to run ./mach eslint --fix <path> and ./mach clang-format -p <path>.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2ad1ef8c60f4
Remove the escaped dot from body lines before saving in the offline store so dot at the beginning of news posts won't be doubled. r=benc

Comment 18

[Magnus Melin [:mkmelin]]

But thanks for the patches! Keep 'em coming! ;)

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 9184980 [details] [diff] [review]
Remove the escaped dot from body lines before saving in the offline store.

[Approval Request Comment]
User impact if declined: per bug summary
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): not very risky, but should be uplifted together with bug 1669277

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 9184980 [details] [diff] [review]
Remove the escaped dot from body lines before saving in the offline store.

[Triage Comment]
Approved for esr78 - needs to be taken with the other patch for 78.
We don't plan another beta for uplifts - so this will appear in beta 84

Comment 21

[Rob Lemley [:rjl]]

Thunderbird 78.5.0:
https://hg.mozilla.org/releases/comm-esr78/rev/744206d92a10