Commit 9cabc799 authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[master] Merge branch 'trac1697'

parents 75ff8831 107995cf
......@@ -307,12 +307,14 @@ makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
for_each(questions.begin(), questions.end(), QuestionInserter(message));
message->setRcode(rcode);
MessageRenderer renderer(*buffer);
MessageRenderer renderer;
renderer.setBuffer(buffer.get());
if (tsig_context.get() != NULL) {
message->toWire(renderer, *tsig_context);
} else {
message->toWire(renderer);
}
renderer.setBuffer(NULL);
LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_ERROR_RESPONSE)
.arg(renderer.getLength()).arg(*message);
}
......@@ -554,7 +556,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
return (true);
}
MessageRenderer renderer(*buffer);
MessageRenderer renderer;
renderer.setBuffer(buffer.get());
const bool udp_buffer =
(io_message.getSocket().getProtocol() == IPPROTO_UDP);
renderer.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
......@@ -563,6 +566,7 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
} else {
message->toWire(renderer);
}
renderer.setBuffer(NULL);
LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
.arg(renderer.getLength()).arg(message->toText());
......@@ -683,12 +687,14 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
message->setHeaderFlag(Message::HEADERFLAG_AA);
message->setRcode(Rcode::NOERROR());
MessageRenderer renderer(*buffer);
MessageRenderer renderer;
renderer.setBuffer(buffer.get());
if (tsig_context.get() != NULL) {
message->toWire(renderer, *tsig_context);
} else {
message->toWire(renderer);
}
renderer.setBuffer(NULL);
return (true);
}
......
......@@ -154,8 +154,7 @@ createBuiltinVersionResponse(const qid_t qid, vector<uint8_t>& data) {
rrset_version_ns->addRdata(generic::NS(version_name));
message.addRRset(Message::SECTION_AUTHORITY, rrset_version_ns);
OutputBuffer obuffer(0);
MessageRenderer renderer(obuffer);
MessageRenderer renderer;
message.toWire(renderer);
data.clear();
......
......@@ -70,8 +70,7 @@ host_lookup(const char* const name, const char* const dns_class,
RRClass(dns_class),
any ? RRType::ANY() : RRType(type))); // if NULL then:
OutputBuffer obuffer(512);
MessageRenderer renderer(obuffer);
MessageRenderer renderer;
msg.toWire(renderer);
struct addrinfo hints, *res;
......@@ -111,7 +110,7 @@ host_lookup(const char* const name, const char* const dns_class,
gettimeofday(&before_time, NULL);
}
sendto(s, obuffer.getData(), obuffer.getLength(), 0, res->ai_addr,
sendto(s, renderer.getData(), renderer.getLength(), 0, res->ai_addr,
res->ai_addrlen);
struct sockaddr_storage ss;
......
......@@ -252,7 +252,8 @@ makeErrorMessage(MessagePtr message, MessagePtr answer_message,
}
for_each(questions.begin(), questions.end(), QuestionInserter(message));
message->setRcode(rcode);
MessageRenderer renderer(*buffer);
MessageRenderer renderer;
renderer.setBuffer(buffer.get());
message->toWire(renderer);
}
......@@ -303,7 +304,8 @@ public:
// Now we can clear the buffer and render the new message into it
buffer->clear();
MessageRenderer renderer(*buffer);
MessageRenderer renderer;
renderer.setBuffer(buffer.get());
ConstEDNSPtr edns(query_message->getEDNS());
const bool dnssec_ok = edns && edns->getDNSSECAwareness();
......@@ -327,6 +329,7 @@ public:
}
answer_message->toWire(renderer);
renderer.setBuffer(NULL);
LOG_DEBUG(resolver_logger, RESOLVER_DBG_DETAIL,
RESOLVER_DNS_MESSAGE_SENT)
......
......@@ -205,7 +205,8 @@ IOFetch::IOFetch(Protocol protocol, IOService& service,
}
void
IOFetch::initIOFetch(MessagePtr& query_msg, Protocol protocol, IOService& service,
IOFetch::initIOFetch(MessagePtr& query_msg, Protocol protocol,
IOService& service,
const isc::dns::Question& question,
const IOAddress& address, uint16_t port,
OutputBufferPtr& buff, Callback* cb, int wait, bool edns)
......@@ -225,8 +226,10 @@ IOFetch::initIOFetch(MessagePtr& query_msg, Protocol protocol, IOService& servic
query_msg->setEDNS(edns_query);
}
MessageRenderer renderer(*data_->msgbuf);
MessageRenderer renderer;
renderer.setBuffer(data_->msgbuf.get());
query_msg->toWire(renderer);
renderer.setBuffer(NULL);
}
// Return protocol in use.
......
......@@ -133,10 +133,15 @@ public:
EDNSPtr msg_edns(new EDNS());
msg_edns->setUDPSize(Message::DEFAULT_MAX_EDNS0_UDPSIZE);
msg.setEDNS(msg_edns);
MessageRenderer renderer(*msgbuf_);
MessageRenderer renderer;
renderer.setBuffer(msgbuf_.get());
msg.toWire(renderer);
renderer.setBuffer(NULL);
renderer.setBuffer(expected_buffer_.get());
msg.toWire(renderer);
MessageRenderer renderer2(*expected_buffer_);
msg.toWire(renderer2);
renderer.setBuffer(NULL);
// Initialize the test data to be returned: tests will return a
// substring of this data. (It's convenient to have this as a member of
......@@ -581,20 +586,22 @@ public:
return_data_ = "Message returned to the client";
udp::endpoint remote;
socket.async_receive_from(asio::buffer(receive_buffer_, sizeof(receive_buffer_)),
remote,
boost::bind(&IOFetchTest::udpReceiveHandler, this, &remote, &socket,
_1, _2, bad_qid, second_send));
socket.async_receive_from(asio::buffer(receive_buffer_,
sizeof(receive_buffer_)),
remote,
boost::bind(&IOFetchTest::udpReceiveHandler,
this, &remote, &socket,
_1, _2, bad_qid, second_send));
service_.get_io_service().post(udp_fetch_);
if (debug_) {
cout << "udpSendReceive: async_receive_from posted, waiting for callback" <<
endl;
cout << "udpSendReceive: async_receive_from posted,"
"waiting for callback" << endl;
}
service_.run();
socket.close();
EXPECT_TRUE(run_);;
EXPECT_TRUE(run_);
}
};
......
......@@ -61,8 +61,7 @@ loadQueryData(istream& input, BenchQueries& queries, const RRClass& qclass,
string line;
unsigned int linenum = 0;
Message query_message(Message::RENDER);
OutputBuffer buffer(128); // this should be sufficiently large
MessageRenderer renderer(buffer);
MessageRenderer renderer;
while (getline(input, line), !input.eof()) {
++linenum;
if (input.bad() || input.fail()) {
......@@ -99,9 +98,9 @@ loadQueryData(istream& input, BenchQueries& queries, const RRClass& qclass,
renderer.clear();
query_message.toWire(renderer);
vector<unsigned char> query_data(
static_cast<const unsigned char*>(buffer.getData()),
static_cast<const unsigned char*>(buffer.getData()) +
buffer.getLength());
static_cast<const unsigned char*>(renderer.getData()),
static_cast<const unsigned char*>(renderer.getData()) +
renderer.getLength());
queries.push_back(query_data);
} catch (const Exception&) {
if (strict) {
......
......@@ -58,7 +58,7 @@ ConstElementPtr SQLITE_DBFILE_EXAMPLE = Element::fromJSON(
class DataSrcTest : public ::testing::Test {
protected:
DataSrcTest() : obuffer(0), renderer(obuffer), msg(Message::PARSE),
DataSrcTest() : msg(Message::PARSE),
opcodeval(Opcode::QUERY().getCode()), qid(0)
{
DataSrcPtr sql3_source = DataSrcPtr(new Sqlite3DataSrc);
......@@ -76,7 +76,6 @@ protected:
HotCache cache;
MetaDataSrc meta_source;
OutputBuffer obuffer;
MessageRenderer renderer;
Message msg;
const uint16_t opcodeval;
......
......@@ -42,7 +42,7 @@ template <typename T>
class RdataRenderBenchMark {
public:
RdataRenderBenchMark(const vector<T>& dataset) :
dataset_(dataset), buffer_(4096), renderer_(buffer_)
dataset_(dataset)
{}
unsigned int run() {
typename vector<T>::const_iterator data;
......@@ -55,7 +55,6 @@ public:
}
private:
const vector<T>& dataset_;
OutputBuffer buffer_;
MessageRenderer renderer_;
};
......
......@@ -12,14 +12,15 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
#include <cctype>
#include <cassert>
#include <set>
#include <exceptions/exceptions.h>
#include <util/buffer.h>
#include <dns/name.h>
#include <dns/messagerenderer.h>
#include <cctype>
#include <cassert>
#include <set>
using namespace isc::util;
namespace isc {
......@@ -171,8 +172,8 @@ struct MessageRenderer::MessageRendererImpl {
CompressMode compress_mode_;
};
MessageRenderer::MessageRenderer(OutputBuffer& buffer) :
AbstractMessageRenderer(buffer),
MessageRenderer::MessageRenderer() :
AbstractMessageRenderer(),
impl_(new MessageRendererImpl)
{}
......@@ -273,9 +274,34 @@ MessageRenderer::writeName(const Name& name, const bool compress) {
}
}
AbstractMessageRenderer::AbstractMessageRenderer() :
local_buffer_(0), buffer_(&local_buffer_)
{
}
void
AbstractMessageRenderer::setBuffer(OutputBuffer* buffer) {
if (buffer != NULL && buffer_->getLength() != 0) {
isc_throw(isc::InvalidParameter,
"MessageRenderer buffer cannot be set when in use");
} if (buffer == NULL && buffer_ == &local_buffer_) {
isc_throw(isc::InvalidParameter,
"Default MessageRenderer buffer cannot be reset");
}
if (buffer == NULL) {
// Reset to the default buffer, then clear other internal resources.
// The order is important; we need to keep the used buffer intact.
buffer_ = &local_buffer_;
clear();
} else {
buffer_ = buffer;
}
}
void
AbstractMessageRenderer::clear() {
buffer_.clear();
buffer_->clear();
}
}
......
......@@ -37,9 +37,15 @@ class Name;
/// comprehensive \c Message class internally; normal applications won't have
/// to care about details of this class.
///
/// Once a renderer class object is constructed with a buffer, it is
/// generally expected that all rendering operations are performed via that
/// object. If the application modifies the buffer in
/// By default any (derived) renderer class object is associated with
/// an internal buffer, and subsequent write operations will be performed
/// on that buffer. The rendering result can be retrieved via the
/// \c getData() method.
///
/// If an application wants a separate buffer can be (normally temporarily)
/// set for rendering operations via the \c setBuffer() method. In that case,
/// it is generally expected that all rendering operations are performed via
/// that object. If the application modifies the buffer in
/// parallel with the renderer, the result will be undefined.
///
/// Note to developers: we introduced a separate class for name compression
......@@ -101,30 +107,30 @@ protected:
///
/// This is intentionally defined as \c protected as this base class should
/// never be instantiated (except as part of a derived class).
/// \param buffer The buffer where the data should be rendered into.
/// \todo We might want to revisit this API at some point and remove the
/// buffer parameter. In that case it would create it's own buffer and
/// a function to extract the data would be available instead. It seems
/// like a cleaner design, but it's left undone until we would actually
/// benefit from the change.
AbstractMessageRenderer(isc::util::OutputBuffer& buffer) :
buffer_(buffer)
{}
AbstractMessageRenderer();
public:
/// \brief The destructor.
virtual ~AbstractMessageRenderer() {}
//@}
protected:
/// \brief Return the output buffer we render into.
const isc::util::OutputBuffer& getBuffer() const { return (buffer_); }
isc::util::OutputBuffer& getBuffer() { return (buffer_); }
const isc::util::OutputBuffer& getBuffer() const { return (*buffer_); }
isc::util::OutputBuffer& getBuffer() { return (*buffer_); }
private:
/// \short Buffer to store data
/// \brief Local (default) buffer to store data.
isc::util::OutputBuffer local_buffer_;
/// \brief Buffer to store data.
///
/// Note that the class interface ensures this pointer is never NULL;
/// it either refers to \c local_buffer_ or to an application-supplied
/// buffer by \c setBuffer().
///
/// It was decided that there's no need to have this in every subclass,
/// at least not now, and this reduces code size and gives compiler a better
/// chance to optimise.
isc::util::OutputBuffer& buffer_;
/// at least not now, and this reduces code size and gives compiler a
/// better chance to optimise.
isc::util::OutputBuffer* buffer_;
public:
///
/// \name Getter Methods
......@@ -136,12 +142,12 @@ public:
/// This method works exactly same as the same method of the \c OutputBuffer
/// class; all notes for \c OutputBuffer apply.
const void* getData() const {
return (buffer_.getData());
return (buffer_->getData());
}
/// \brief Return the length of data written in the internal buffer.
size_t getLength() const {
return (buffer_.getLength());
return (buffer_->getLength());
}
/// \brief Return whether truncation has occurred while rendering.
......@@ -175,6 +181,35 @@ public:
/// \name Setter Methods
///
//@{
/// \brief Set or reset a temporary output buffer.
///
/// This method can be used for an application that manages an output
/// buffer separately from the message renderer and wants to keep reusing
/// the renderer. When the renderer is associated with the default buffer
/// and the given pointer is non NULL, the given buffer will be
/// (temporarily) used for subsequent message rendering; if the renderer
/// is associated with a temporary buffer and the given pointer is NULL,
/// the renderer will be reset with the default buffer. In the latter
/// case any additional resources (possibly specific to a derived renderer
/// class) will be cleared, but the temporary buffer is kept as the latest
/// state (which would normally store the rendering result).
///
/// This method imposes some restrictions to prevent accidental misuse
/// that could cause disruption such as dereferencing an invalid object.
/// First, a temporary buffer must not be set when the associated buffer
/// is in use, that is, any data are stored in the buffer. Also, the
/// default buffer cannot be "reset"; when NULL is specified a temporary
/// buffer must have been set beforehand. If these conditions aren't met
/// an isc::InvalidParameter exception will be thrown. This method is
/// exception free otherwise.
///
/// \throw isc::InvalidParameter A restrictions of the method usage isn't
/// met.
///
/// \param buffer A pointer to a temporary output buffer or NULL for reset
/// it.
void setBuffer(isc::util::OutputBuffer* buffer);
/// \brief Mark the renderer to indicate truncation has occurred while
/// rendering.
///
......@@ -209,7 +244,7 @@ public:
///
/// \param len The length of the gap to be inserted in bytes.
void skip(size_t len) {
buffer_.skip(len);
buffer_->skip(len);
}
/// \brief Trim the specified length of data from the end of the internal
......@@ -223,7 +258,7 @@ public:
///
/// \param len The length of data that should be trimmed.
void trim(size_t len) {
buffer_.trim(len);
buffer_->trim(len);
}
/// \brief Clear the internal buffer and other internal resources.
......@@ -236,7 +271,7 @@ public:
///
/// \param data The 8-bit integer to be written into the internal buffer.
void writeUint8(const uint8_t data) {
buffer_.writeUint8(data);
buffer_->writeUint8(data);
}
/// \brief Write an unsigned 16-bit integer in host byte order into the
......@@ -244,7 +279,7 @@ public:
///
/// \param data The 16-bit integer to be written into the buffer.
void writeUint16(uint16_t data) {
buffer_.writeUint16(data);
buffer_->writeUint16(data);
}
/// \brief Write an unsigned 16-bit integer in host byte order at the
......@@ -259,7 +294,7 @@ public:
/// \param data The 16-bit integer to be written into the internal buffer.
/// \param pos The beginning position in the buffer to write the data.
void writeUint16At(uint16_t data, size_t pos) {
buffer_.writeUint16At(data, pos);
buffer_->writeUint16At(data, pos);
}
/// \brief Write an unsigned 32-bit integer in host byte order into the
......@@ -267,7 +302,7 @@ public:
///
/// \param data The 32-bit integer to be written into the buffer.
void writeUint32(uint32_t data) {
buffer_.writeUint32(data);
buffer_->writeUint32(data);
}
/// \brief Copy an arbitrary length of data into the internal buffer
......@@ -278,7 +313,7 @@ public:
/// \param data A pointer to the data to be copied into the internal buffer.
/// \param len The length of the data in bytes.
void writeData(const void *data, size_t len) {
buffer_.writeData(data, len);
buffer_->writeData(data, len);
}
/// \brief Write a \c Name object into the internal buffer in wire format,
......@@ -316,10 +351,7 @@ public:
using AbstractMessageRenderer::CASE_SENSITIVE;
/// \brief Constructor from an output buffer.
///
/// \param buffer An \c OutputBuffer object to which wire format data is
/// written.
MessageRenderer(isc::util::OutputBuffer& buffer);
MessageRenderer();
virtual ~MessageRenderer();
virtual bool isTruncated() const;
......
......@@ -36,7 +36,6 @@ namespace {
class s_MessageRenderer : public PyObject {
public:
s_MessageRenderer();
isc::util::OutputBuffer* outputbuffer;
MessageRenderer* cppobj;
};
......@@ -78,17 +77,14 @@ PyMethodDef MessageRenderer_methods[] = {
int
MessageRenderer_init(s_MessageRenderer* self) {
self->outputbuffer = new OutputBuffer(4096);
self->cppobj = new MessageRenderer(*self->outputbuffer);
self->cppobj = new MessageRenderer;
return (0);
}
void
MessageRenderer_destroy(s_MessageRenderer* self) {
delete self->cppobj;
delete self->outputbuffer;
self->cppobj = NULL;
self->outputbuffer = NULL;
Py_TYPE(self)->tp_free(self);
}
......
......@@ -70,8 +70,7 @@ namespace {
// it's hopefully an acceptable practice.
class RdataFieldComposer : public AbstractMessageRenderer {
public:
RdataFieldComposer(OutputBuffer& buffer) :
AbstractMessageRenderer(buffer),
RdataFieldComposer() :
truncated_(false), length_limit_(65535),
mode_(CASE_INSENSITIVE), last_data_pos_(0)
{}
......@@ -128,8 +127,7 @@ public:
}
RdataFields::RdataFields(const Rdata& rdata) {
OutputBuffer buffer(0);
RdataFieldComposer field_composer(buffer);
RdataFieldComposer field_composer;
rdata.toWire(field_composer);
nfields_ = field_composer.getFields().size();
data_length_ = field_composer.getLength();
......
......@@ -43,9 +43,7 @@ const uint8_t EDNS::SUPPORTED_VERSION;
namespace {
class EDNSTest : public ::testing::Test {
protected:
EDNSTest() : rrtype(RRType::OPT()), buffer(NULL, 0), obuffer(0),
renderer(obuffer), rcode(0)
{
EDNSTest() : rrtype(RRType::OPT()), buffer(NULL, 0), obuffer(0), rcode(0) {
opt_rdata = ConstRdataPtr(new generic::OPT());
edns_base.setUDPSize(4096);
}
......
......@@ -79,7 +79,6 @@ namespace {
class MessageTest : public ::testing::Test {
protected:
MessageTest() : test_name("test.example.com"), obuffer(0),
renderer(obuffer),
message_parse(Message::PARSE),
message_render(Message::RENDER),
bogus_section(static_cast<Message::Section>(
......@@ -731,8 +730,8 @@ TEST_F(MessageTest, toWire) {
message_render.toWire(renderer);
vector<unsigned char> data;
UnitTestUtil::readWireData("message_toWire1", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
obuffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageTest, toWireInParseMode) {
......@@ -961,9 +960,6 @@ TEST_F(MessageTest, toWireTSIGNoTruncation) {
// rendering fail unexpectedly in the test that follows.
class BadRenderer : public MessageRenderer {
public:
BadRenderer(isc::util::OutputBuffer& buffer) :
MessageRenderer(buffer)
{}
virtual void setLengthLimit(size_t len) {
if (getLength() > 0) {
MessageRenderer::setLengthLimit(getLength());
......@@ -994,8 +990,7 @@ TEST_F(MessageTest, toWireTSIGLengthErrors) {
InvalidParameter);
// Trying to render a message with TSIG using a buggy renderer.
obuffer.clear();
BadRenderer bad_renderer(obuffer);
BadRenderer bad_renderer;
bad_renderer.setLengthLimit(512);
message_render.clear(Message::RENDER);
EXPECT_THROW(commonTSIGToWireCheck(message_render, bad_renderer, tsig_ctx,
......
......@@ -12,8 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
#include <vector>
#include <exceptions/exceptions.h>
#include <util/buffer.h>
#include <dns/name.h>
#include <dns/messagerenderer.h>
......@@ -22,6 +21,8 @@
#include <gtest/gtest.h>
#include <vector>
using isc::UnitTestUtil;
using isc::dns::Name;
using isc::dns::MessageRenderer;
......@@ -30,15 +31,13 @@ using isc::util::OutputBuffer;
namespace {
class MessageRendererTest : public ::testing::Test {
protected:
MessageRendererTest() : expected_size(0), buffer(0), renderer(buffer)
{
MessageRendererTest() : expected_size(0) {
data16 = (2 << 8) | 3;
data32 = (4 << 24) | (5 << 16) | (6 << 8) | 7;
}