Fix a renderer crash while processing FTP directory listing.

net/ftp/ftp_directory_listing_buffer.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.  Use of this
 // source code is governed by a BSD-style license that can be found in the
 // LICENSE file.
 
 #include "net/ftp/ftp_directory_listing_buffer.h"
 
 #include "base/i18n/icu_encoding_detection.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
@@ skipping 98 matching lines @@
   buffer_.erase(0, cut_pos);
   return OK;
 }
 
 int FtpDirectoryListingBuffer::ParseLines() {
   while (!lines_.empty()) {
     string16 line = lines_.front();
     lines_.pop_front();
     if (current_parser_) {
       if (!current_parser_->ConsumeLine(line))
         return ERR_FAILED;

[wtc, 2010/06/26 17:44:37]

Should we return ERR_UNRECOGNIZED_FTP_DIRECTORY_LISTING_FORMAT
instead?

[Paweł Hajdan Jr., 2010/06/28 16:58:49]

Indeed, getting rid of ERR_FAILED would be good. I'm going to prepare a
follow-up CL. I'd just rather add ERR_INVALID_FTP_DIRECTORY_LISTING_FORMAT so
the code is more specific. When current_parser_ is non-null, it indicates it
"thinks" it recognized the format.

[wtc, 2010/06/30 15:07:53]

Adding ERR_INVALID_FTP_DIRECTORY_LISTING_FORMAT is fine.

     } else {
       ParserSet::iterator i = parsers_.begin();
       while (i != parsers_.end()) {
         if ((*i)->ConsumeLine(line)) {
           i++;
         } else {
           delete *i;
           parsers_.erase(i++);
         }
       }
       if (parsers_.empty())
         return ERR_UNRECOGNIZED_FTP_DIRECTORY_LISTING_FORMAT;
       if (parsers_.size() == 1)
         current_parser_ = *parsers_.begin();
     }
   }
 
   return OK;
 }
 
 int FtpDirectoryListingBuffer::OnEndOfInput() {
   ParserSet::iterator i = parsers_.begin();
   while (i != parsers_.end()) {
     if ((*i)->OnEndOfInput()) {
       i++;
     } else {
       delete *i;

[wtc, 2010/06/26 17:44:37]

You can also fix this bug by adding the following code
before deleting |*i|:
      if (current_parser_ == *i)
        current_parser_ = NULL;

Your fix is fine.  I just wanted to check my understanding
of the code.

[Paweł Hajdan Jr., 2010/06/28 16:58:49]

Sounds good to me.

       parsers_.erase(i++);
     }
   }
 
   if (parsers_.size() != 1) {
-    DCHECK(!current_parser_);
+    current_parser_ = NULL;
 
     // We may hit an ambiguity in case of listings which have no entries. That's
     // fine, as long as all remaining parsers agree that the listing is empty.
     bool all_listings_empty = true;
     for (ParserSet::iterator i = parsers_.begin(); i != parsers_.end(); ++i) {
       if ((*i)->EntryAvailable())
         all_listings_empty = false;
     }
     if (all_listings_empty)
       return OK;
 
     return ERR_UNRECOGNIZED_FTP_DIRECTORY_LISTING_FORMAT;
   }
 
   current_parser_ = *parsers_.begin();
   return OK;
 }
 
 }  // namespace net