Browse Source

[1539] handle the case where session push fails.

JINMEI Tatuya 13 years ago
parent
commit
b726fc55cd

+ 14 - 2
src/bin/auth/auth_srv.cc

@@ -130,6 +130,13 @@ public:
         }
     }
 
+    void close() {
+        if (connected_) {
+            forwarder_.close();
+            connected_ = false;
+        }
+    }
+
     // Push a socket session corresponding to given IOMessage.
     //
     // NOTE: Right now, there's no API to retrieve the local address from
@@ -816,8 +823,13 @@ AuthSrvImpl::processUpdate(const IOMessage& io_message,
                            OutputBuffer& /*buffer*/,
                            std::auto_ptr<TSIGContext> /*tsig_context*/)
 {
-    ddns_forwarder_.connect();
-    ddns_forwarder_.push(io_message);
+    try {
+        ddns_forwarder_.connect();
+        ddns_forwarder_.push(io_message);
+    } catch (const SocketSessionError& ex) {
+        ddns_forwarder_.close();
+        throw;
+    }
 
     return (false);
 }

+ 23 - 0
src/bin/auth/tests/auth_srv_unittest.cc

@@ -1499,6 +1499,29 @@ TEST_F(AuthSrvTest, DDNSForwardConnectFail) {
     EXPECT_TRUE(ddns_forwarder.isConnected());
 }
 
+TEST_F(AuthSrvTest, DDNSForwardPushFail) {
+    // Make first request succeed, which will establish the connection.
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    createAndSendRequest(RRType::SOA(), Opcode::UPDATE());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+
+    // make connect attempt fail.  It should result in SERVFAIL.  The
+    // connection should be closed.
+    ddns_forwarder.disablePush();
+    createAndSendRequest(RRType::SOA(), Opcode::UPDATE());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::SERVFAIL(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+
+    // Allow push again.  Connection will be reopened, and the request will
+    // be forwarded successfully.
+    ddns_forwarder.enablePush();
+    createAndSendRequest(RRType::SOA(), Opcode::UPDATE());
+    EXPECT_FALSE(dnsserv.hasAnswer());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+}
+
 TEST_F(AuthSrvTest, DDNSForwardClose) {
     scoped_ptr<AuthSrv> tmp_server(new AuthSrv(true, xfrout, ddns_forwarder));
     UnitTestUtil::createRequestMessage(request_message, Opcode::UPDATE(),

+ 8 - 0
src/lib/util/unittests/mock_socketsession.h

@@ -52,6 +52,9 @@ public:
         is_connected_ = true;
     }
     virtual void close() {
+        if (!is_connected_) {
+            isc_throw(isc::util::io::SocketSessionError, "duplicate close");
+        }
         is_connected_ = false;
     }
 
@@ -68,6 +71,10 @@ public:
             isc_throw(isc::util::io::SocketSessionError,
                        "socket session forwarding is disabled for test");
         }
+        if (!is_connected_) {
+            isc_throw(isc::util::io::SocketSessionError,
+                       "socket session is being pushed before connected");
+        }
 
         // Copy parameters for later checks
         pushed_sock_ = sock;
@@ -96,6 +103,7 @@ public:
     void disableClose() { close_ok_ = false; }
     void enableClose() { close_ok_ = true; }
     void disablePush() { push_ok_ = false; }
+    void enablePush() { push_ok_ = true; }
 
     // Read-only accessors to recorded parameters to the previous successful
     // call to push().  Return values are undefined if there has been no