Limited NSEC support to RecordParsed

net/dns/record_rdata.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/dns/record_rdata.h"
 
 #include "net/base/big_endian.h"
 #include "net/base/dns_util.h"
 #include "net/dns/dns_protocol.h"
 #include "net/dns/dns_response.h"
@@ skipping 197 matching lines @@
 uint16 TxtRecordRdata::Type() const {
   return TxtRecordRdata::kType;
 }
 
 bool TxtRecordRdata::IsEqual(const RecordRdata* other) const {
   if (other->Type() != Type()) return false;
   const TxtRecordRdata* txt_other = static_cast<const TxtRecordRdata*>(other);
   return texts_ == txt_other->texts_;
 }
 
+NsecRecordRdata::NsecRecordRdata() {
+}
+
+NsecRecordRdata::~NsecRecordRdata() {
+}
+
+// static
+scoped_ptr<NsecRecordRdata> NsecRecordRdata::Create(
+    const base::StringPiece& data,
+    const DnsRecordParser& parser) {
+  scoped_ptr<NsecRecordRdata> rdata(new NsecRecordRdata);
+
+  unsigned len_next_domain = parser.ReadName(data.data(), NULL);

[szym, 2013/06/24 21:50:28]

nit: |next_domain_len| would be better, |next_domain_length| even better.

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+
+  if (len_next_domain == 0 || data.length() < len_next_domain + 2)
+    return scoped_ptr<NsecRecordRdata>();
+

[szym, 2013/06/24 21:50:28]

StringPiece::operator[] returns char, so I'd suggest something like:

struct BitmapHeader {
  uint8 block_number;
  uint8 length;
} header = reinterpret_cast<BitmapHeader*>(
    data.data() + next_domain_length);

if (header->block_number)
  return ...();  // Block number must be zero.

Another option is:
unsigned bitmap_length = static_cast<unsigned>(data[...]);

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+  unsigned window_num = data[len_next_domain];

[szym, 2013/06/24 21:50:28]

I suggest block_num or block_number
"window number" is used only once in RFC3845 and RFC6762 does not use "window"
at all.

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+  unsigned len = data[len_next_domain+1];

[szym, 2013/06/24 21:50:28]

bitmap_length

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+
+  // The window number must be zero in mDns-secific nsec records. The bitmap

[szym, 2013/06/24 21:50:28]

"specific"
"NSEC"

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+  // length must be between 1 and 32, and since we may only have one window, the
+  // data length must be exactl equal to the domain length plus bitmap size.

[szym, 2013/06/24 21:50:28]

"exactly"

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+  if (window_num != 0 || len == 0 || len > 32 ||
+      data.length() != len_next_domain + 2 + len)

[szym, 2013/06/24 21:50:28]

The last check would be easier to understand if you moved it after the
construction of the bitmap_data StringPiece.

if (bitmap_data.length() != bitmap_length)
  return ..();

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+    return scoped_ptr<NsecRecordRdata>();
+
+  base::StringPiece bitmap_data = data.substr(len_next_domain + 2);
+
+  rdata->bitmap_.insert(rdata->bitmap_.begin(),
+                     bitmap_data.begin(),

[gene, 2013/06/24 19:47:16]

indentation

[Noam Samuel, 2013/06/24 21:31:30]

Done.

+                     bitmap_data.end());
+
+  return rdata.Pass();
+}
+
+uint16 NsecRecordRdata::Type() const {
+  return NsecRecordRdata::kType;
+}
+
+bool NsecRecordRdata::IsEqual(const RecordRdata* other) const {
+  if (other->Type() != Type()) return false;

[gene, 2013/06/24 19:47:16]

move return to the next line

[Noam Samuel, 2013/06/24 21:31:30]

Done.

+  const NsecRecordRdata* nsec_other =
+      static_cast<const NsecRecordRdata*>(other);
+  return bitmap_ == nsec_other->bitmap_;
+}
+
+bool NsecRecordRdata::bit(unsigned i) {
+  unsigned byte_num = i/8;
+  if (bitmap_.size() < byte_num + 1)
+    return false;
+
+  unsigned bit_num = 7 - i % 8;
+  return (bitmap_[byte_num] & (1 << bit_num)) != 0;
+}
+
 }  // namespace net