504726 - tm_year off by 1900 bugs in ParseFTPList.cpp

Status: RESOLVED WONTFIX

(Core Graveyard :: Networking: FTP, defect)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

This bug is similar to the second issue that Michal Novotny
fixed in bug 365133 comment 3, but was found by my code inspection.

Since ParseFTPList.cpp handles all kinds of FTP servers, it's
hard to find the particular kinds of FTP servers affected by
this bug, so I'm afraid that the correctness of my patch needs
to be determined by code review.

I am submitting this patch in fulfillment of the requirements of
MPL section 3.2 because Chromium's src/net/ftp/ftp_directory_parser.cc
is based on ParseFTPList.cpp.

Comment 1

[Michal Novotny [:michal]]

Comment on attachment 389074 [details] [diff] [review]
Proposed patch

Looks good. Would be nice to have a testcase...

Comment 2

[Jason Duell]

Comment on attachment 389074 [details] [diff] [review]
Proposed patch

Sorry, I'm not a superreviewer.  Assigning to biesi.  If you don't get a response from him, try bz (note: this is a fairly small patch, so it shouldn't take too much time).

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 389074 [details] [diff] [review]
Proposed patch

I love how this code is inconsistent :/
in one place:
           if (result->fe_time.tm_year < 70)
             result->fe_time.tm_year += 100;

in another place:
          if (result->fe_time.tm_year < 80)
            result->fe_time.tm_year += 2000;

Comment 4

[Patrick McManus [:mcmanus]]

We are in a period where ftp is clearly deprecated and in general, making changes to the code is riskier than letting it ride unless there is a patch and reviewer available to make a good judgment about it. So I'm going to wontfix ftp bugs related to enhancements, interop errors, etc.. We will be better off putting our energy into including a different js based ftp stack.