Merge branch 'trac956'

src/bin/auth/auth_srv.cc

@@ -442,12 +442,9 @@ makeErrorMessage(MessageRenderer& renderer, Message& message,
     message.setRcode(rcode);
 
     RendererHolder holder(renderer, &buffer, stats_attrs);
-    if (tsig_context.get() != NULL) {
-        message.toWire(renderer, *tsig_context);
-        stats_attrs.setResponseTSIG(true);
-    } else {
-        message.toWire(renderer);
-    }
+    message.toWire(renderer, tsig_context.get());
+    stats_attrs.setResponseTSIG(tsig_context.get() != NULL);
+
     LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_ERROR_RESPONSE)
               .arg(renderer.getLength()).arg(message);
 }
@@ -674,12 +671,9 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message,
     const bool udp_buffer =
         (io_message.getSocket().getProtocol() == IPPROTO_UDP);
     renderer_.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
-    if (tsig_context.get() != NULL) {
-        message.toWire(renderer_, *tsig_context);
-        stats_attrs.setResponseTSIG(true);
-    } else {
-        message.toWire(renderer_);
-    }
+    message.toWire(renderer_, tsig_context.get());
+    stats_attrs.setResponseTSIG(tsig_context.get() != NULL);
+
     LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
               .arg(renderer_.getLength()).arg(message);
     return (true);
@@ -836,12 +830,8 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
     message.setRcode(Rcode::NOERROR());
 
     RendererHolder holder(renderer_, &buffer, stats_attrs);
-    if (tsig_context.get() != NULL) {
-        message.toWire(renderer_, *tsig_context);
-        stats_attrs.setResponseTSIG(true);
-    } else {
-        message.toWire(renderer_);
-    }
+    message.toWire(renderer_, tsig_context.get());
+    stats_attrs.setResponseTSIG(tsig_context.get() != NULL);
     return (true);
 }
 

src/bin/ddns/ddns.py.in

@@ -495,10 +495,7 @@ class DDNSServer:
             return False
         msg = update_session.get_message()
         self.__response_renderer.clear()
-        if tsig_ctx is not None:
-            msg.to_wire(self.__response_renderer, tsig_ctx)
-        else:
-            msg.to_wire(self.__response_renderer)
+        msg.to_wire(self.__response_renderer, tsig_ctx)
 
         ret = self.__send_response(sock, self.__response_renderer.get_data(),
                                    remote_addr)

src/bin/ddns/tests/ddns_test.py

@@ -900,10 +900,7 @@ def create_msg(opcode=Opcode.UPDATE, zones=[TEST_ZONE_RECORD], prereq=[],
         msg.add_rrset(SECTION_PREREQUISITE, p)
 
     renderer = MessageRenderer()
-    if tsigctx is not None:
-        msg.to_wire(renderer, tsigctx)
-    else:
-        msg.to_wire(renderer)
+    msg.to_wire(renderer, tsigctx)
 
     # re-read the created data in the parse mode
     msg.clear(Message.PARSE)

src/bin/xfrin/tests/xfrin_test.py

@@ -373,10 +373,7 @@ class MockXfrinConnection(XfrinConnection):
         [resp.add_rrset(Message.SECTION_AUTHORITY, a) for a in authorities]
 
         renderer = MessageRenderer()
-        if tsig_ctx is not None:
-            resp.to_wire(renderer, tsig_ctx)
-        else:
-            resp.to_wire(renderer)
+        resp.to_wire(renderer, tsig_ctx)
         reply_data = struct.pack('H', socket.htons(renderer.get_length()))
         reply_data += renderer.get_data()
 

src/bin/xfrout/xfrout.py.in

@@ -345,12 +345,7 @@ class XfroutSession():
         render.set_compress_mode(MessageRenderer.CASE_SENSITIVE)
         render.set_length_limit(XFROUT_MAX_MESSAGE_SIZE)
 
-        # XXX Currently, python wrapper doesn't accept 'None' parameter in this case,
-        # we should remove the if statement and use a universal interface later.
-        if tsig_ctx is not None:
-            msg.to_wire(render, tsig_ctx)
-        else:
-            msg.to_wire(render)
+        msg.to_wire(render, tsig_ctx)
 
         header_len = struct.pack('H', socket.htons(render.get_length()))
         self._send_data(sock_fd, header_len)

src/lib/dns/message.cc

@@ -604,13 +604,8 @@ Message::addQuestion(const Question& question) {
 }
 
 void
-Message::toWire(AbstractMessageRenderer& renderer) {
-    impl_->toWire(renderer, NULL);
-}
-
-void
-Message::toWire(AbstractMessageRenderer& renderer, TSIGContext& tsig_ctx) {
-    impl_->toWire(renderer, &tsig_ctx);
+Message::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
+    impl_->toWire(renderer, tsig_ctx);
 }
 
 void

src/lib/dns/message.h

@@ -550,29 +550,18 @@ public:
     std::string toText() const;
 
     /// \brief Render the message in wire formant into a message renderer
-    /// object.
+    /// object with (or without) TSIG.
     ///
     /// This \c Message must be in the \c RENDER mode and both \c Opcode and
     /// \c Rcode must have been set beforehand; otherwise, an exception of
     /// class \c InvalidMessageOperation will be thrown.
     ///
-    /// \note The renderer's internal buffers and data are automatically
-    /// cleared, keeping the length limit and the compression mode intact.
-    /// In case truncation is triggered, the renderer is cleared completely.
-    ///
-    /// \param renderer DNS message rendering context that encapsulates the
-    /// output buffer and name compression information.
-    void toWire(AbstractMessageRenderer& renderer);
-
-    /// \brief Render the message in wire formant into a message renderer
-    /// object with TSIG.
-    ///
-    /// This method is similar to the other version of \c toWire(), but
-    /// it will also add a TSIG RR with (in many cases) the TSIG MAC for
-    /// the message along with the given TSIG context (\c tsig_ctx).
-    /// The TSIG RR will be placed at the end of \c renderer.
-    /// \c tsig_ctx will be updated based on the fact it was used for signing
-    /// and with the latest MAC.
+    /// If a non-NULL \c tsig_ctx is passed, it will also add a TSIG RR
+    /// with (in many cases) the TSIG MAC for the message along with the
+    /// given TSIG context (\c tsig_ctx).  The TSIG RR will be placed at
+    /// the end of \c renderer. The \c TSIGContext at \c tsig_ctx will
+    /// be updated based on the fact it was used for signing and with
+    /// the latest MAC.
     ///
     /// \exception InvalidMessageOperation The message is not in the Render
     /// mode, or either Rcode or Opcode is not set.
@@ -589,10 +578,12 @@ public:
     /// cleared, keeping the length limit and the compression mode intact.
     /// In case truncation is triggered, the renderer is cleared completely.
     ///
-    /// \param renderer See the other version
+    /// \param renderer DNS message rendering context that encapsulates the
+    /// output buffer and name compression information.
     /// \param tsig_ctx A TSIG context that is to be used for signing the
     /// message
-    void toWire(AbstractMessageRenderer& renderer, TSIGContext& tsig_ctx);
+    void toWire(AbstractMessageRenderer& renderer,
+                TSIGContext* tsig_ctx = NULL);
 
     /// Parse options.
     ///

src/lib/dns/python/message_python.cc

@@ -696,10 +696,10 @@ Message_toWire(s_Message* self, PyObject* args) {
     PyObject* mr;
     PyObject* tsig_ctx = NULL;
 
-    if (PyArg_ParseTuple(args, "O!|O!", &messagerenderer_type, &mr,
-                         &tsigcontext_type, &tsig_ctx)) {
+    if (PyArg_ParseTuple(args, "O!|O", &messagerenderer_type, &mr,
+                         &tsig_ctx)) {
         try {
-            if (tsig_ctx == NULL) {
+            if ((tsig_ctx == NULL) || (tsig_ctx == Py_None)) {
                 self->cppobj->toWire(PyMessageRenderer_ToMessageRenderer(mr));
             } else {
                 self->cppobj->toWire(PyMessageRenderer_ToMessageRenderer(mr),
@@ -726,7 +726,7 @@ Message_toWire(s_Message* self, PyObject* args) {
     }
     PyErr_Clear();
     PyErr_SetString(PyExc_TypeError,
-                    "toWire argument must be a MessageRenderer");
+                    "Bad to_wire() arguments were passed");
     return (NULL);
 }
 

src/lib/dns/python/tests/message_python_test.py

@@ -371,6 +371,13 @@ class MessageTest(unittest.TestCase):
         self.__common_tsigmessage_setup()
         self.__common_tsig_checks("message_toWire2.wire")
 
+    def test_to_wire_with_tsig_none(self):
+        message_render = create_message()
+        renderer = MessageRenderer()
+        message_render.to_wire(renderer, None)
+        self.assertEqual(b'\x105\x85\x00\x00\x01\x00\x02\x00\x00\x00\x00\x04test\x07example\x03com\x00\x00\x01\x00\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x02',
+                         renderer.get_data())
+
     def test_to_wire_with_edns_tsig(self):
         fix_current_time(0x4db60d1f)
         self.r.set_qid(0x6cd)

src/lib/dns/python/tsig_python.cc

@@ -334,14 +334,21 @@ PyTSIGContext_Check(PyObject* obj) {
     return (PyObject_TypeCheck(obj, &tsigcontext_type));
 }
 
-TSIGContext&
+TSIGContext*
 PyTSIGContext_ToTSIGContext(PyObject* tsigcontext_obj) {
     if (tsigcontext_obj == NULL) {
         isc_throw(PyCPPWrapperException,
                   "obj argument NULL in TSIGContext PyObject conversion");
     }
+
+    if (!PyTSIGContext_Check(tsigcontext_obj)) {
+        isc_throw(TSIGContextError,
+                  "obj argument is of wrong type in TSIGContext "
+                  "PyObject conversion");
+    }
+
     s_TSIGContext* tsigcontext = static_cast<s_TSIGContext*>(tsigcontext_obj);
-    return (*tsigcontext->cppobj);
+    return (tsigcontext->cppobj);
 }
 
 } // namespace python

src/lib/dns/python/tsig_python.h

@@ -36,8 +36,8 @@ extern PyObject* po_TSIGContextError;
 /// \return true if the object is of type TSIGContext, false otherwise
 bool PyTSIGContext_Check(PyObject* obj);
 
-/// \brief Returns a reference to the TSIGContext object contained within the given
-///        Python object.
+/// \brief Returns a pointer to the TSIGContext object contained within
+///        the given Python object.
 ///
 /// \note The given object MUST be of type TSIGContext; this can be checked with
 ///       either the right call to ParseTuple("O!"), or with PyTSIGContext_Check()
@@ -46,7 +46,7 @@ bool PyTSIGContext_Check(PyObject* obj);
 /// may be destroyed, the caller must copy it itself.
 ///
 /// \param tsigcontext_obj The tsigcontext object to convert
-TSIGContext& PyTSIGContext_ToTSIGContext(PyObject* tsigcontext_obj);
+TSIGContext* PyTSIGContext_ToTSIGContext(PyObject* tsigcontext_obj);
 
 
 } // namespace python

src/lib/dns/tests/message_unittest.cc

@@ -893,7 +893,7 @@ commonTSIGToWireCheck(Message& message, MessageRenderer& renderer,
         message.addRRset(Message::SECTION_ANSWER, ans_rrset);
     }
 
-    message.toWire(renderer, tsig_ctx);
+    message.toWire(renderer, &tsig_ctx);
     vector<unsigned char> expected_data;
     UnitTestUtil::readWireData(expected_file, expected_data);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
@@ -1026,7 +1026,7 @@ TEST_F(MessageTest, toWireTSIGTruncation3) {
         message_render.addQuestion(Question(Name("www.example.com"),
                                             RRClass::IN(), RRType(i)));
     }
-    message_render.toWire(renderer, tsig_ctx);
+    message_render.toWire(renderer, &tsig_ctx);
 
     // Check the rendered data by parsing it.  We only check it has the
     // TC bit on, has the correct number of questions, and has a TSIG RR.

src/lib/dns/tests/tsig_unittest.cc

@@ -188,7 +188,7 @@ TSIGTest::createMessageAndSign(uint16_t id, const Name& qname,
         (ctx->getState() == TSIGContext::INIT) ?
         TSIGContext::SENT_REQUEST : TSIGContext::SENT_RESPONSE;
 
-    message.toWire(renderer, *ctx);
+    message.toWire(renderer, ctx);
 
     message.clear(Message::PARSE);
     InputBuffer buffer(renderer.getData(), renderer.getLength());

src/lib/testutils/srv_test.cc

@@ -72,12 +72,7 @@ SrvTestBase::createRequestPacket(Message& message,
                                  const char* const remote_address,
                                  uint16_t remote_port)
 {
-    if (context == NULL) {
-        message.toWire(request_renderer);
-    } else {
-        message.toWire(request_renderer, *context);
-    }
-
+    message.toWire(request_renderer, context);
     endpoint.reset(IOEndpoint::create(protocol, IOAddress(remote_address),
                                       remote_port));
     io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() :