Compatibility updates for "ls -l" style FTP LIST response parser:

Compatibility updates for "ls -l" style FTP LIST response parser:

- recognize permission listing with ACL indicator as valid
- add one more test for non-ASCII encoding

TEST=Covered by net_unittests.
BUG=25520
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34240

[eroman, 2009-12-09 22:11:38]

lgtm

[wtc, 2009-12-09 23:02:15]

http://codereview.chromium.org/472003/diff/1/7
File net/ftp/ftp_directory_listing_parser_ls.cc (right):

http://codereview.chromium.org/472003/diff/1/7#newcode42
net/ftp/ftp_directory_listing_parser_ls.cc:42: text.substr(10).empty() ||
text.substr(10) == ASCIIToUTF16("+"));
Do you need parentheses around
  text.substr(10).empty() || text.substr(10) == ASCIIToUTF16("+")
?

&& has a higher precedence than ||.

[Paweł Hajdan Jr., 2009-12-10 07:50:40]

On 2009/12/09 23:02:15, wtc wrote:
> http://codereview.chromium.org/472003/diff/1/7
> File net/ftp/ftp_directory_listing_parser_ls.cc (right):
> 
> http://codereview.chromium.org/472003/diff/1/7#newcode42
> net/ftp/ftp_directory_listing_parser_ls.cc:42: text.substr(10).empty() ||
> text.substr(10) == ASCIIToUTF16("+"));
> Do you need parentheses around
>   text.substr(10).empty() || text.substr(10) == ASCIIToUTF16("+")
> ?
> 
> && has a higher precedence than ||.

Indeed, thanks for catching that. I added a test case for that bug and committed
the patch.