[trac499] Final tidy-up before review

src/lib/asiolink/tests/io_fetch_unittest.cc

@@ -19,7 +19,7 @@
 
 #include <gtest/gtest.h>
 #include <boost/bind.hpp>
-
+#include <boost/date_time/posix_time/posix_time_types.hpp>
 
 #include <asio.hpp>
 
@@ -47,7 +47,8 @@ namespace asiolink {
 const asio::ip::address TEST_HOST(asio::ip::address::from_string("127.0.0.1"));
 const uint16_t TEST_PORT(5301);
 // FIXME Shouldn't we send something that is real message?
-const char TEST_DATA[] = "Test response from server to client";
+const char TEST_DATA[] = "Test response from server to client (longer than 30 bytes)";
+const int SEND_INTERVAL = 500;   // Interval in ms between TCP sends
 
 /// \brief Test fixture for the asiolink::IOFetch.
 class IOFetchTest : public virtual ::testing::Test, public virtual IOFetch::Callback
@@ -63,11 +64,15 @@ public:
     IOFetch         tcp_fetch_;     ///< For TCP query test
     IOFetch::Protocol protocol_;    ///< Protocol being tested
     size_t          cumulative_;    ///< Cumulative data received by "server".
+    deadline_timer  timer_;         ///< Timer to measure timeouts
 
     // The next member is the buffer in which the "server" (implemented by the
     // response handler methods in this class) receives the question sent by the
     // fetch object.
-    uint8_t         server_buff_[512];  ///< Server buffer
+    uint8_t         receive_buffer_[512];   ///< Server receive buffer
+    uint8_t         send_buffer_[512];      ///< Server send buffer
+    uint16_t        send_size_;             ///< Amount of data to sent
+    uint16_t        send_cumulative_;       ///< Data sent so far
 
     /// \brief Constructor
     IOFetchTest() :
@@ -80,9 +85,15 @@ public:
         udp_fetch_(IOFetch::UDP, service_, question_, IOAddress(TEST_HOST),
             TEST_PORT, result_buff_, this, 100),
         tcp_fetch_(IOFetch::TCP, service_, question_, IOAddress(TEST_HOST),
-            TEST_PORT, result_buff_, this, 1000),
+            TEST_PORT, result_buff_, this, (4 * SEND_INTERVAL)),
+                                        // Timeout interval chosen to ensure no timeout
         protocol_(IOFetch::TCP),        // for initialization - will be changed
-        cumulative_(0)
+        cumulative_(0),
+        timer_(service_.get_io_service()),
+        receive_buffer_(),
+        send_buffer_(),
+        send_size_(0),
+        send_cumulative_(0)
     {
         // Construct the data buffer for question we expect to receive.
         Message msg(Message::RENDER);
@@ -112,12 +123,12 @@ public:
         // The QID in the incoming data is random so set it to 0 for the
         // data comparison check. (It is set to 0 in the buffer containing
         // the expected data.)
-        server_buff_[0] = server_buff_[1] = 0;
+        receive_buffer_[0] = receive_buffer_[1] = 0;
 
         // Check that length of the received data and the expected data are
         // identical, then check that the data is identical as well.
         EXPECT_EQ(msgbuf_->getLength(), length);
-        EXPECT_TRUE(equal(server_buff_, (server_buff_ + length - 1),
+        EXPECT_TRUE(equal(receive_buffer_, (receive_buffer_ + length - 1),
         static_cast<const uint8_t*>(msgbuf_->getData())));
 
         // Return a message back to the IOFetch object.
@@ -138,7 +149,7 @@ public:
 
         // Initiate a read on the socket.
         cumulative_ = 0;
-        socket->async_receive(asio::buffer(server_buff_, sizeof(server_buff_)),
+        socket->async_receive(asio::buffer(receive_buffer_, sizeof(receive_buffer_)),
             boost::bind(&IOFetchTest::tcpReceiveHandler, this, socket, _1, _2));
     }
 
@@ -163,13 +174,13 @@ public:
         cumulative_ += length;
         bool complete = false;
         if (cumulative_ > 2) {
-            uint16_t dns_length = readUint16(server_buff_);
+            uint16_t dns_length = readUint16(receive_buffer_);
             complete = ((dns_length + 2) == cumulative_);
         }
 
         if (!complete) {
-            socket->async_receive(asio::buffer((server_buff_ + cumulative_),
-                (sizeof(server_buff_) - cumulative_)),
+            socket->async_receive(asio::buffer((receive_buffer_ + cumulative_),
+                (sizeof(receive_buffer_) - cumulative_)),
                 boost::bind(&IOFetchTest::tcpReceiveHandler, this, socket, _1, _2));
             return;
         }
@@ -180,24 +191,44 @@ public:
         // field the QID in the received buffer is in the third and fourth
         // bytes.
         EXPECT_EQ(msgbuf_->getLength() + 2, cumulative_);
-        server_buff_[2] = server_buff_[3] = 0;
-        EXPECT_TRUE(equal((server_buff_ + 2), (server_buff_ + cumulative_ - 2),
+        receive_buffer_[2] = receive_buffer_[3] = 0;
+        EXPECT_TRUE(equal((receive_buffer_ + 2), (receive_buffer_ + cumulative_ - 2),
             static_cast<const uint8_t*>(msgbuf_->getData())));
 
         // ... and return a message back.  This has to be preceded by a two-byte
-        // count field.  It's simpler to do this as two writes - it shouldn't
-        // make any difference to the IOFetch object.
-        //
-        // When specifying the callback handler, the expected size of the
-        // data written is passed as the first parameter.
-        uint8_t count[2];
-        writeUint16(sizeof(TEST_DATA), count);
-        socket->async_send(asio::buffer(count, 2),
-                           boost::bind(&IOFetchTest::tcpSendHandler, this,
-                                       2, _1, _2));
-        socket->async_send(asio::buffer(TEST_DATA, sizeof(TEST_DATA)),
+        // count field.  Construct the message.
+        assert(sizeof(send_buffer_) > (sizeof(TEST_DATA) + 2));
+        writeUint16(sizeof(TEST_DATA), send_buffer_);
+        copy(TEST_DATA, TEST_DATA + sizeof(TEST_DATA) - 1, send_buffer_ + 2);
+        send_size_ = sizeof(TEST_DATA) + 2;
+
+        // Send the data.  This is done in multiple writes with a delay between
+        // each to check that the reassembly of TCP packets from fragments works.
+        send_cumulative_ = 0;
+        tcpSendData(socket);
+    }
+
+    /// \brief Sent Data Over TCP
+    ///
+    /// Send the TCP data back to the IOFetch object.  The data is sent in
+    /// three chunks - two of 16 bytes and the remainder, with a 250ms gap
+    /// between each.
+    ///
+    /// \param socket Socket over which send should take place
+    void tcpSendData(tcp::socket* socket) {
+        // Decide what to send based on the cumulative count
+        uint8_t* send_ptr = &send_buffer_[send_cumulative_];
+                                    // Pointer to data to send
+        size_t amount = 16;         // Amount of data to send
+        if (send_cumulative_ > 30) {
+            amount = send_size_ - send_cumulative_;
+        }
+
+        // ... and send it.  The amount sent is also passed as the first argument
+        // of the send callback, as a check.
+        socket->async_send(asio::buffer(send_ptr, amount),
                            boost::bind(&IOFetchTest::tcpSendHandler, this,
-                                       sizeof(TEST_DATA), _1, _2));
+                                       amount, socket, _1, _2));
     }
 
     /// \brief Completion Handler for Sending TCP data
@@ -207,14 +238,35 @@ public:
     /// be asynchronous because control needs to return to the caller in order
     /// for the IOService "run()" method to be called to run the handlers.)
     ///
+    /// If not all the data has been sent, a short delay is instigated (during
+    /// which control returns to the IOService).  This should force the queued
+    /// data to actually be sent and the IOFetch receive handler to be triggered.
+    /// In this way, the ability of IOFetch to handle fragmented TCP packets
+    /// should be checked.
+    ///
     /// \param expected Number of bytes that were expected to have been sent.
+    /// \param socket Socket over which the send took place.  Only used to
+    ///        pass back to the send method.
     /// \param ec Boost error code, value should be zero.
     /// \param length Number of bytes sent.
-    void tcpSendHandler(size_t expected = 0, error_code ec = error_code(),
-                        size_t length = 0)
+    void tcpSendHandler(size_t expected, tcp::socket* socket,
+                        error_code ec = error_code(), size_t length = 0)
     {
         EXPECT_EQ(0, ec.value());       // Expect no error
         EXPECT_EQ(expected, length);    // And that amount sent is as expected
+
+        // Do we need to send more?
+        send_cumulative_ += length;
+        if (send_cumulative_ < send_size_) {
+
+            // Yes - set up a timer:  the callback handler for the timer is
+            // tcpSendData, which will then send the next chunk.  We pass the
+            // socket over which data should be sent as an argument to that
+            // function.
+            timer_.expires_from_now(boost::posix_time::milliseconds(SEND_INTERVAL));
+            timer_.async_wait(boost::bind(&IOFetchTest::tcpSendData, this,
+                                          socket));
+        }
     }
 
     /// \brief Fetch completion callback
@@ -343,7 +395,7 @@ TEST_F(IOFetchTest, UdpSendReceive) {
     socket.bind(udp::endpoint(TEST_HOST, TEST_PORT));
 
     udp::endpoint remote;
-    socket.async_receive_from(asio::buffer(server_buff_, sizeof(server_buff_)),
+    socket.async_receive_from(asio::buffer(receive_buffer_, sizeof(receive_buffer_)),
         remote,
         boost::bind(&IOFetchTest::udpReceiveHandler, this, &remote, &socket,
                     _1, _2));

src/lib/asiolink/tests/recursive_query_unittest_2.cc

@@ -77,12 +77,11 @@ namespace asiolink {
 const std::string TEST_ADDRESS = "127.0.0.1";   ///< Servers are on this address
 const uint16_t TEST_PORT = 5301;                ///< ... and this port
 const size_t BUFFER_SIZE = 1024;                ///< For all buffers
-const char* WWW_EXAMPLE_ORG = "192.0.2.254";    ///< Answer to question
+const char* WWW_EXAMPLE_ORG = "192.0.2.254";    ///< Address of www.example.org
 
 // As the test is fairly long and complex, debugging "print" statements have
 // been left in although they are disabled.  Set the following to "true" to
 // enable them.
-
 const bool DEBUG_PRINT = false;
 
 /// \brief Test fixture for the RecursiveQuery Test
@@ -126,7 +125,6 @@ public:
     OutputBufferPtr udp_send_buffer_;           ///< Send buffer for UDP I/O
     udp::socket     udp_socket_;                ///< Socket used by UDP server
 
-
     /// \brief Constructor
     RecursiveQueryTest2() :
         debug_(DEBUG_PRINT),
@@ -147,9 +145,7 @@ public:
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
         udp_socket_(service_.get_io_service(), udp::v4())
-    {
-
-    }
+    {}
 
     /// \brief Set Common Message Bits
     ///
@@ -178,8 +174,9 @@ public:
         }
 
         // Do a referral to org.  We'll define all NS records as "in-zone"
-        // nameservers (and so supply glue) to avoid the possibility of
-        // the resolver doing another lookup.
+        // nameservers (and supply glue) to avoid the possibility of the
+        // resolver starting another recursive query to resolve the address of
+        // a nameserver.
         RRsetPtr org_ns(new RRset(Name("org."), RRClass::IN(), RRType::NS(), RRTTL(300)));
         org_ns->addRdata(createRdata(RRType::NS(), RRClass::IN(), "ns1.org."));
         org_ns->addRdata(createRdata(RRType::NS(), RRClass::IN(), "ns2.org."));
@@ -206,8 +203,9 @@ public:
         }
 
         // Do a referral to example.org.  As before, we'll define all NS
-        // records as "in-zone" nameservers (and so supply glue) to avoid
-        // the possibility of the resolver doing another lookup.
+        // records as "in-zone" nameservers (and supply glue) to avoid the
+        // possibility of the resolver starting another recursive query to look
+        // up the address of the nameserver.
         RRsetPtr example_org_ns(new RRset(Name("example.org."), RRClass::IN(), RRType::NS(), RRTTL(300)));
         example_org_ns->addRdata(createRdata(RRType::NS(), RRClass::IN(), "ns1.example.org."));
         example_org_ns->addRdata(createRdata(RRType::NS(), RRClass::IN(), "ns2.example.org."));
@@ -224,7 +222,7 @@ public:
 
     /// \brief Set Answer to "www.example.org"
     ///
-    /// Sets up the passed-in message (expected to be in "RENDER" mode to
+    /// Sets up the passed-in message (expected to be in "RENDER" mode) to
     /// indicate an authoritative answer to www.example.org.
     ///
     /// \param msg Message to update with referral information.
@@ -245,9 +243,10 @@ public:
 
     /// \brief UDP Receive Handler
     ///
-    /// This is invoked when a message is received from the RecursiveQuery
-    /// Object.  It formats an answer and sends it, with the UdpSendHandler
-    /// method being specified as the completion handler.
+    /// This is invoked when a message is received over UDP from the
+    /// RecursiveQuery object under test.  It formats an answer and sends it
+    /// asynchronously, with the UdpSendHandler method being specified as the
+    /// completion handler.
     ///
     /// \param ec ASIO error code, completion code of asynchronous I/O issued
     ///        by the "server" to receive data.
@@ -270,11 +269,7 @@ public:
         udp_receive_buffer_[0] = udp_receive_buffer_[1] = 0;
 
         // Check that question we received is what was expected.
-        Message received_message(Message::PARSE);
-        InputBuffer received_buffer(udp_receive_buffer_, length);
-        received_message.fromWire(received_buffer);
-        Question received_question = **(received_message.beginQuestion());
-        EXPECT_TRUE(received_question == *question_);
+        checkReceivedPacket(udp_receive_buffer_, length);
 
         // The message returned depends on what state we are in.  Set up
         // common stuff first: bits not mentioned are set to 0.
@@ -316,15 +311,14 @@ public:
         MessageRenderer renderer(*udp_send_buffer_);
         msg.toWire(renderer);
 
-        // Return a message back to the IOFetch object.
+        // Return a message back to the IOFetch object (after setting the
+        // expected length of data for the check in the send handler).
+        udp_length_ = udp_send_buffer_->getLength();
         udp_socket_.async_send_to(asio::buffer(udp_send_buffer_->getData(),
                                                udp_send_buffer_->getLength()),
                                   udp_remote_,
                                   boost::bind(&RecursiveQueryTest2::udpSendHandler,
                                               this, _1, _2));
-
-        // Set the expected length for the send handler.
-        udp_length_ = udp_send_buffer_->getLength();
     }
 
     /// \brief UDP Send Handler
@@ -332,6 +326,9 @@ public:
     /// Called when a send operation of the UDP server (i.e. a response
     /// being sent to the RecursiveQuery) has completed, this re-issues
     /// a read call.
+    ///
+    /// \param ec Completion error code of the send.
+    /// \param length Actual number of bytes sent.
     void udpSendHandler(error_code ec = error_code(), size_t length = 0) {
         if (debug_) {
             cout << "udpSendHandler(): error = " << ec.value() <<
@@ -342,7 +339,7 @@ public:
         EXPECT_EQ(0, ec.value());
         EXPECT_EQ(udp_length_, length);
 
-        // Reissue the receive.
+        // Reissue the receive call to await the next message.
         udp_socket_.async_receive_from(
             asio::buffer(udp_receive_buffer_, sizeof(udp_receive_buffer_)),
             udp_remote_,
@@ -351,7 +348,7 @@ public:
 
     /// \brief Completion Handler for Accepting TCP Data
     ///
-    /// Called when the remote system connects to the "server".  It issues
+    /// Called when the remote system connects to the "TCP server".  It issues
     /// an asynchronous read on the socket to read data.
     ///
     /// \param socket Socket on which data will be received
@@ -376,7 +373,8 @@ public:
     /// \brief Completion Handler for Receiving TCP Data
     ///
     /// Reads data from the RecursiveQuery object and loops, reissuing reads,
-    /// until all the message has been read.  It then sends
+    /// until all the message has been read.  It then returns an appropriate
+    /// response.
     ///
     /// \param socket Socket to use to send the answer
     /// \param ec ASIO error code, completion code of asynchronous I/O issued
@@ -422,11 +420,7 @@ public:
 
         // Check that question we received is what was expected.  Note that we
         // have to ignore the two-byte header in order to parse the message.
-        Message received_message(Message::PARSE);
-        InputBuffer received_buffer(tcp_receive_buffer_ + 2, tcp_cumulative_ - 2);
-        received_message.fromWire(received_buffer);
-        Question received_question = **(received_message.beginQuestion());
-        EXPECT_TRUE(received_question == *question_);
+        checkReceivedPacket(tcp_receive_buffer_ + 2, length - 2);
 
         // Return a message back.  This is a referral to example.org, which
         // should result in another query over UDP.  Note the setting of the
@@ -443,9 +437,10 @@ public:
         // Expected next state (when checked) is the UDP query to example.org.
         // Also, take this opportunity to clear the accumulated read count in
         // readiness for the next read. (If any - at present, there is only
-        // one read in the test,, although extensions to this test suite could
+        // one read in the test, although extensions to this test suite could
         // change that.)
         expected_ = UDP_EXAMPLE_ORG;
+        tcp_cumulative_ = 0;
 
         // We'll write the message in two parts, the count and the message
         // itself. This saves having to prepend the count onto the start of a
@@ -485,6 +480,28 @@ public:
         EXPECT_EQ(expected_length, length);    // And that amount sent is as expected
     }
 
+    /// \brief Check Received Packet
+    ///
+    /// Checks the packet received from the RecursiveQuery object to ensure
+    /// that the question is what is expected.
+    ///
+    /// \param data Start of data.  This is the start of the received buffer in
+    ///        the case of UDP data, and an offset into the buffer past the
+    ///        count field for TCP data.
+    /// \param length Length of data.
+    void checkReceivedPacket(uint8_t* data, size_t length) {
+
+        // Decode the received buffer.
+        InputBuffer buffer(data, length);
+        Message message(Message::PARSE);
+        message.fromWire(buffer);
+
+        // Check the packet.
+        EXPECT_FALSE(message.getHeaderFlag(Message::HEADERFLAG_QR));
+
+        Question question = **(message.beginQuestion());
+        EXPECT_TRUE(question == *question_);
+    }
 };
 
 /// \brief Resolver Callback Object