206214 - Wrong parsing of output from german MS ftp server

Status: RESOLVED WONTFIX

(Core Graveyard :: Networking: FTP, defect)

Description

[Roy-Magne Mo]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030518
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030518

Mozilla ftp client seems to parse dates wrong with the listings from Microsoft
FTP Service (Version 4.0)

Reproducible: Always

Steps to Reproduce:
1.enter url 
2.
3.

Actual Results:  
Index of ftp://www.rittal.de/
public 		13. mar 103 	77:41:44

Expected Results:  
Actual output with a regular ftp client shows:
03-12-03  10:06AM       <DIR>          public

so I guess the corres output should be:
public         13. mar 2003    10:06:00

Comment 1

[Andrew Schultz]

regression between builds 2002082722 and 2002082904
see also bug 170070 and bug 183963, which regressed at the same point with
similar symptoms (wrong year).
bug 95590 is the apparent culprit

Comment 2

[tenthumbs]

Attachment: patch to nsFTPDirListingConv.cpp

It probably better to find and fix all the cases in
mozilla/netwerk/streamconv/converters/ParseFTPList.cpp but this untested patch
should catch this situation.

Comment 3

[Doug Turner (:dougt)]

tenthumbs, are we erronously substracting 1900 in ParseFTPList?

Comment 4

[tenthumbs]

Actually, we're adding 100, at line 805.

     805           if (result->fe_time.tm_year < 80)
     806             result->fe_time.tm_year += 100;

That's not an unreasonable thing so maybe there's supposed to be another 
place that adds 1900. Maybe it got lost. There's also this:

     665           if (result->fe_time.tm_year < 70)
     666             result->fe_time.tm_year += 100;

There are lots of places that subtract 1900 but I don't think they apply 
here.

It's just way too complicated for my feeble mind so I took the easy way 
out.

Comment 5

[Doug Turner (:dougt)]

cyrus, do you know of similar problems or do you think this could have been my
porting the time code to NSPR.

Comment 6

[Cyrus Patel]

Assuming that the members of NSPRs PRExplodedTime structure have the same
meaning as the members of the libc 'struct tm' (and everything I've seen
indicates that this assumption is correct), then....

the tm_year member is the number of years _since_ 1900, not a year number.

a) *adding* 1900 in nsFTPDirListingConv.cpp (or anywhere else really)
   would not be correct.
   http://bugzilla.mozilla.org/attachment.cgi?id=123705&action=view   
   is wrong. 
   Replace the call to PR_FormatTimeUSEnglish() with a call to 
   PR_FormatTime() [which it used to be] and the result would
   be correct.

b) *subtracting* 1900 _is_ valid if the file's date was a year number.
   (file's date already includes a century).
   http://bugzilla.mozilla.org/attachment.cgi?id=100231&action=view is wrong!

c) There is a bug in PR_FormatTimeUSEnglish()
   [mozilla/source/nsprpub/pr/src/misc/prtime.c]

   1894  case 'Y':
   1895       /* year as ccyy ( for example 1986 ) */
   1896       PR_snprintf(tmpBuf,tmpBufSize,"%.4d",time->tm_year );
   1897       ADDSTR( bufPtr, bufSize, tmpBuf ); 
   1898       break;

   Line 1896 should read 
         PR_snprintf(tmpBuf,tmpBufSize,"%.4d",time->tm_year + 1900 );

Comment 7

[tenthumbs]

From prtime.h,

    PRInt16 tm_year;            /* absolute year, AD (note that we do 
                                   not count from 1900) */

Maybe I stumbled into the right fix.

Comment 8

[Cyrus Patel]

yep, that it is. :)

Comment 9

[Doug Turner (:dougt)]

mass reassigning to nobody.

Comment 10

[Matthias Versen [:Matti]]

moving to NSOR according to comment#6

Comment 11

[Wan-Teh Chang]

(In reply to Cyrus Patel from comment #6)
> Assuming that the members of NSPRs PRExplodedTime structure have the same
> meaning as the members of the libc 'struct tm' (and everything I've seen
> indicates that this assumption is correct), then....

This assumption is wrong.
 
> the tm_year member is the number of years _since_ 1900, not a year number.

The tm_year member of the PRExplodedTime structure is a year number.

> c) There is a bug in PR_FormatTimeUSEnglish()
>    [mozilla/source/nsprpub/pr/src/misc/prtime.c]
> 
>    1894  case 'Y':
>    1895       /* year as ccyy ( for example 1986 ) */
>    1896       PR_snprintf(tmpBuf,tmpBufSize,"%.4d",time->tm_year );
>    1897       ADDSTR( bufPtr, bufSize, tmpBuf ); 
>    1898       break;
> 
>    Line 1896 should read 
>          PR_snprintf(tmpBuf,tmpBufSize,"%.4d",time->tm_year + 1900 );

The current code is correct. We should not add 1900 to time->tm_year.

Comment 12

[Matthias Versen [:Matti]]

thanks wtc, moving back to core:networking FTP

The URL from comment#0 seems to work fine in recent builds.

Comment 13

[Patrick McManus [:mcmanus]]

only critical ftp fixes now