[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);
-
-    return rv;
+    return received_malformed_response_ ? ERR_DNS_MALFORMED_RESPONSE : rv;

[mmenke, 2012/08/28 14:15:54]

Err...How does this work?  It looks like if we ever get a malformed response, we
set |received_malformed_response_| to true, and never switch it to false if we
subsequently get a non-malformed response, we'll still return
ERR_DNS_MALFORMED_RESPONSE here.

[szym, 2012/08/28 14:59:44]

You're right. This isn't finished yet, and the test I wrote doesn't really
exercise it. I'll upload a fixed patch soon.

   }
 
   int DoConnect() {
     next_state_ = STATE_SEND_QUERY;
     return socket_->Connect(server_);
   }
 
   int DoSendQuery() {
     next_state_ = STATE_SEND_QUERY_COMPLETE;
     return socket_->Write(query_->io_buffer(),
@@ skipping 24 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 ERR_IO_PENDING;
     }
     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 344 matching lines @@
 }  // namespace
 
 // static
 scoped_ptr<DnsTransactionFactory> DnsTransactionFactory::CreateFactory(
     DnsSession* session) {
   return scoped_ptr<DnsTransactionFactory>(
       new DnsTransactionFactoryImpl(session));
 }
 
 }  // namespace net