[net/dns] Don't abandon a DnsUDPAttempt when the response does not match the query.

net/dns/dns_transaction.cc

 // Copyright (c) 2012 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/dns/dns_transaction.h"
 
 #include <deque>
 #include <string>
 #include <vector>
 
@@ skipping 53 matching lines @@
 // A single asynchronous DNS exchange over UDP, which consists of sending out a
 // DNS query, waiting for a response, and returning the response that it
 // matches. Logging is done in the socket and in the outer DnsTransaction.
 class DnsUDPAttempt {
  public:
   DnsUDPAttempt(scoped_ptr<DatagramClientSocket> socket,
                 const IPEndPoint& server,
                 scoped_ptr<DnsQuery> query,
                 const CompletionCallback& callback)
       : next_state_(STATE_NONE),
+        received_malformed_response_(false),
         socket_(socket.Pass()),
         server_(server),
         query_(query.Pass()),
         callback_(callback) {
   }
 
   // Starts the attempt. Returns ERR_IO_PENDING if cannot complete synchronously
   // and calls |callback| upon completion.
   int Start() {
     DCHECK_EQ(STATE_NONE, next_state_);
@@ skipping 59 matching lines @@
           rv = DoReadResponse();
           break;
         case STATE_READ_RESPONSE_COMPLETE:
           rv = DoReadResponseComplete(rv);
           break;
         default:
           NOTREACHED();
           break;
       }
     } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
-
+    // If we received a malformed response, and are now waiting for another one,
+    // indicate to the transaction that the server might be misbehaving.
+    if (rv == ERR_IO_PENDING && received_malformed_response_)
+      return ERR_DNS_MALFORMED_RESPONSE;
     return rv;
   }
 
   int DoConnect() {
     next_state_ = STATE_SEND_QUERY;
     return socket_->Connect(server_);
   }
 
   int DoSendQuery() {
     next_state_ = STATE_SEND_QUERY_COMPLETE;
@@ skipping 25 matching lines @@
                                     base::Unretained(this)));
   }
 
   int DoReadResponseComplete(int rv) {
     DCHECK_NE(ERR_IO_PENDING, rv);
     if (rv < 0)
       return rv;
 
     DCHECK(rv);
     if (!response_->InitParse(rv, *query_)) {
-      // TODO(szym): Consider making this reaction less aggressive.
       // Other implementations simply ignore mismatched responses. Since each
       // DnsUDPAttempt binds to a different port, we might find that responses
       // to previously timed out queries lead to failures in the future.
-      // http://crbug.com/107413
-      return ERR_DNS_MALFORMED_RESPONSE;
+      // Our solution is to make another attempt, in case the query truly
+      // failed, but keep this attempt alive, in case it was a false alarm.
+      received_malformed_response_ = true;
+      next_state_ = STATE_READ_RESPONSE;
+      return OK;
     }
     if (response_->flags() & dns_protocol::kFlagTC)
       return ERR_DNS_SERVER_REQUIRES_TCP;
     // TODO(szym): Extract TTL for NXDOMAIN results. http://crbug.com/115051
     if (response_->rcode() == dns_protocol::kRcodeNXDOMAIN)
       return ERR_NAME_NOT_RESOLVED;
     if (response_->rcode() != dns_protocol::kRcodeNOERROR)
       return ERR_DNS_SERVER_FAILED;
 
     CHECK(response());
     return OK;
   }
 
   void OnIOComplete(int rv) {
     rv = DoLoop(rv);
     if (rv != ERR_IO_PENDING)
       callback_.Run(rv);
   }
 
   State next_state_;
+  bool received_malformed_response_;
 
   scoped_ptr<DatagramClientSocket> socket_;
   IPEndPoint server_;
   scoped_ptr<DnsQuery> query_;
 
   scoped_ptr<DnsResponse> response_;
 
   CompletionCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsUDPAttempt);
@@ skipping 45 matching lines @@
     return qtype_;
   }
 
   virtual int Start() OVERRIDE {
     DCHECK(!callback_.is_null());
     DCHECK(attempts_.empty());
     net_log_.BeginEvent(NetLog::TYPE_DNS_TRANSACTION,
                         base::Bind(&NetLogStartCallback, &hostname_, qtype_));
     int rv = PrepareSearch();
     if (rv == OK) {
-      AttemptResult result = FinishAttempt(StartQuery());
+      AttemptResult result = ProcessAttemptResult(StartQuery());
       if (result.rv == OK) {
         // DnsTransaction must never succeed synchronously.
         MessageLoop::current()->PostTask(
             FROM_HERE,
             base::Bind(&DnsTransactionImpl::DoCallback, AsWeakPtr(), result));
         return ERR_IO_PENDING;
       }
       rv = result.rv;
     }
     if (rv != ERR_IO_PENDING) {
@@ skipping 143 matching lines @@
 
     attempts_.clear();
     return MakeAttempt();
   }
 
   void OnAttemptComplete(unsigned attempt_number, int rv) {
     if (callback_.is_null())
       return;
     DCHECK_LT(attempt_number, attempts_.size());
     const DnsUDPAttempt* attempt = attempts_[attempt_number];
-    AttemptResult result = FinishAttempt(AttemptResult(rv, attempt));
+    AttemptResult result = ProcessAttemptResult(AttemptResult(rv, attempt));
     if (result.rv != ERR_IO_PENDING)
       DoCallback(result);
   }
 
   void LogResponse(const DnsUDPAttempt* attempt) {
     if (attempt && attempt->response()) {
       net_log_.AddEvent(
           NetLog::TYPE_DNS_TRANSACTION_RESPONSE,
           base::Bind(&DnsUDPAttempt::NetLogResponseCallback,
                      base::Unretained(attempt)));
     }
   }
 
   bool MoreAttemptsAllowed() const {
     const DnsConfig& config = session_->config();
     return attempts_.size() < config.attempts * config.nameservers.size();
   }
 
   // Resolves the result of a DnsUDPAttempt until a terminal result is reached
   // or it will complete asynchronously (ERR_IO_PENDING).
-  AttemptResult FinishAttempt(AttemptResult result) {
+  AttemptResult ProcessAttemptResult(AttemptResult result) {
     while (result.rv != ERR_IO_PENDING) {
       LogResponse(result.attempt);
 
       switch (result.rv) {
         case OK:
           net_log_.EndEventWithNetErrorCode(
               NetLog::TYPE_DNS_TRANSACTION_QUERY, result.rv);
           DCHECK(result.attempt);
           DCHECK(result.attempt->response());
           return result;
@@ skipping 17 matching lines @@
           break;
         default:
           // Server failure.
           DCHECK(result.attempt);
           if (result.attempt != attempts_.back()) {
             // This attempt already timed out. Ignore it.
             return AttemptResult(ERR_IO_PENDING, NULL);
           }
           if (MoreAttemptsAllowed()) {
             result = MakeAttempt();
+          } else if (result.rv == ERR_DNS_MALFORMED_RESPONSE) {
+            // Wait until the last attempt times out.
+            return AttemptResult(ERR_IO_PENDING, NULL);
           } else {
             return AttemptResult(result.rv, NULL);
           }
           break;
       }
     }
     return result;
   }
 
   void OnTimeout() {
     if (callback_.is_null())
       return;
-    AttemptResult result = FinishAttempt(
+    AttemptResult result = ProcessAttemptResult(
         AttemptResult(ERR_DNS_TIMED_OUT, NULL));
     if (result.rv != ERR_IO_PENDING)
       DoCallback(result);
   }
 
   scoped_refptr<DnsSession> session_;
   std::string hostname_;
   uint16 qtype_;
   // Cleared in DoCallback.
   DnsTransactionFactory::CallbackType callback_;
@@ skipping 43 matching lines @@
 }  // namespace
 
 // static
 scoped_ptr<DnsTransactionFactory> DnsTransactionFactory::CreateFactory(
     DnsSession* session) {
   return scoped_ptr<DnsTransactionFactory>(
       new DnsTransactionFactoryImpl(session));
 }
 
 }  // namespace net