BIND 10 trac871, updated. 89fcb0f002447e2cc3148e212f1ecd4f6d1bd821 [trac871] avoid using a hardcoded magic number in a test case
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed May 4 17:07:30 UTC 2011
The branch, trac871 has been updated
via 89fcb0f002447e2cc3148e212f1ecd4f6d1bd821 (commit)
via 28c50de2e9ef07250b54a3d99850ee353f511ffe (commit)
via 297955d6928e3202c25d0f1fa17708c032c46a40 (commit)
via 2979d921553eaaf59cfbe6b78bd726e09006ca9e (commit)
from 31a6f34026c83594bd9f205df18eb290114d9ea1 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 89fcb0f002447e2cc3148e212f1ecd4f6d1bd821
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 4 10:06:15 2011 -0700
[trac871] avoid using a hardcoded magic number in a test case
commit 28c50de2e9ef07250b54a3d99850ee353f511ffe
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 4 09:54:11 2011 -0700
[trac871] removed an assertion on arcount after inserting TSIG on further
thought with review comment. update the code comment accordingly.
in fact overflow could (probably) happen if the caller of toWire() tries to
render to many RRsets with an invalidly large buffer size limit. also,
the risk of overflow is not specific to this case; it can happen after
inserting ordinary RRs. So if we want to care about such cases we should
rather cover all of them than making a TSIG specifici check. For now,
I don't bother to check such stupid attempt of the caller. It would simply
result in bogus wire data and wouldn't make a program crash or something.
commit 297955d6928e3202c25d0f1fa17708c032c46a40
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 4 09:41:52 2011 -0700
[trac871] added more description for the RenderSection struct.
commit 2979d921553eaaf59cfbe6b78bd726e09006ca9e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 4 09:28:51 2011 -0700
[trac871] use TODO instead of TBD for consistency
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/message.cc | 29 ++++++++++++++++++++++-------
src/lib/dns/tests/tsigrecord_unittest.cc | 9 ++++-----
2 files changed, 26 insertions(+), 12 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/message.cc b/src/lib/dns/message.cc
index 1d43637..770698a 100644
--- a/src/lib/dns/message.cc
+++ b/src/lib/dns/message.cc
@@ -168,6 +168,22 @@ MessageImpl::setRcode(const Rcode& rcode) {
}
namespace {
+// This helper class is used by MessageImpl::toWire() to render a set of
+// RRsets of a specific section of message to a given MessageRenderer.
+//
+// A RenderSection object is expected to be used with a QuestionIterator or
+// SectionIterator. Its operator() is called for each RRset as the iterator
+// iterates over the corresponding section, and it renders the RRset to
+// the given MessageRenderer, while counting the number of RRs (note: not
+// RRsets) successfully rendered. If the MessageRenderer reports the need
+// for truncation (via its isTruncated() method), the RenderSection object
+// stops rendering further RRsets. In addition, unless partial_ok (given on
+// construction) is true, it removes any RRs that are partially rendered
+// from the MessageRenderer.
+//
+// On the completion of rendering the entire section, the owner of the
+// RenderSection object can get the number of rendered RRs via the
+// getTotalCount() method.
template <typename T>
struct RenderSection {
RenderSection(AbstractMessageRenderer& renderer, const bool partial_ok) :
@@ -219,7 +235,7 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
for_each(questions_.begin(), questions_.end(),
RenderSection<QuestionPtr>(renderer, false)).getTotalCount();
- // TBD: sort RRsets in each section based on configuration policy.
+ // TODO: sort RRsets in each section based on configuration policy.
uint16_t ancount = 0;
if (!renderer.isTruncated()) {
ancount =
@@ -278,7 +294,7 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
codes_and_flags |= (flags_ & HEADERFLAG_MASK);
renderer.writeUint16At(codes_and_flags, header_pos);
header_pos += sizeof(uint16_t);
- // XXX: should avoid repeated pattern (TODO)
+ // TODO: should avoid repeated pattern
renderer.writeUint16At(qdcount, header_pos);
header_pos += sizeof(uint16_t);
renderer.writeUint16At(ancount, header_pos);
@@ -288,15 +304,14 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
renderer.writeUint16At(arcount, header_pos);
// Add TSIG, if necessary, at the end of the message.
- // TBD: truncate case consideration
+ // TODO: truncate case consideration
if (tsig_ctx != NULL) {
tsig_ctx->sign(qid_, renderer.getData(),
renderer.getLength())->toWire(renderer);
- // update the ARCOUNT for the TSIG RR
- ++arcount;
- assert(arcount != 0); // this should never happen for a sane message
- renderer.writeUint16At(arcount, header_pos);
+ // update the ARCOUNT for the TSIG RR. Note that for a sane DNS
+ // message arcount should never overflow to 0.
+ renderer.writeUint16At(++arcount, header_pos);
}
}
diff --git a/src/lib/dns/tests/tsigrecord_unittest.cc b/src/lib/dns/tests/tsigrecord_unittest.cc
index 1610df1..c1223ee 100644
--- a/src/lib/dns/tests/tsigrecord_unittest.cc
+++ b/src/lib/dns/tests/tsigrecord_unittest.cc
@@ -58,7 +58,7 @@ TEST_F(TSIGRecordTest, getName) {
}
TEST_F(TSIGRecordTest, getLength) {
- // 85 = 17 + 26 + 16 + 24
+ // 85 = 17 + 26 + 16 + 26
// len(www.example.com) = 17
// len(hmac-md5.sig-alg.reg.int) = 26
// len(MAC) = 16
@@ -82,10 +82,9 @@ TEST_F(TSIGRecordTest, recordToWire) {
}
TEST_F(TSIGRecordTest, recordToOLongToWire) {
- // Rendering the test record requires a room of 85 bytes (see the
- // getLength test). By setting the limit to 84, it will fail, and
- // the renderer will be marked as "truncated".
- renderer.setLengthLimit(84);
+ // By setting the limit to "record length - 1", it will fail, and the
+ // renderer will be marked as "truncated".
+ renderer.setLengthLimit(test_record.getLength() - 1);
EXPECT_FALSE(renderer.isTruncated()); // not marked before render attempt
EXPECT_EQ(0, test_record.toWire(renderer));
EXPECT_TRUE(renderer.isTruncated());
More information about the bind10-changes
mailing list