From: Tomek Mrugalski <tomasz@isc.org>
Date: Mon, 6 Oct 2014 18:20:48 +0000 (+0200)
Subject: [3546] Yet another review changes:
X-Git-Tag: trac3489_base~2^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=936d262dbe4ff22b1e058767d12a3102cd025024;p=thirdparty%2Fkea.git

[3546] Yet another review changes:

 - duplicate warning removed.
 - unpackVendorOptions4 now returns proper offset when option is truncated
---

diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 56b022fee0..9b100a9c39 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -433,7 +433,12 @@ size_t LibDHCP::unpackVendorOptions6(const uint32_t vendor_id,
 
         if (offset + opt_len > length) {
             // @todo: consider throwing exception here.
-            return (offset);
+
+            // We peeked at the option header of the next option, but discovered
+            // that it would end up beyond buffer end, so the option is
+            // truncated. Hence we can't parse it. Therefore we revert
+            // back by those four bytes (as if we never parsed them).
+            return (offset - 4);
         }
 
         OptionPtr opt;
@@ -509,8 +514,13 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
         uint8_t data_len = buf[offset++];
 
         if (offset + data_len > buf.size()) {
-            // Truncated data-option
-            return (offset);
+            // The option is truncated.
+
+            // We peeked at the data_len, but discovered that it would end up
+            // beyond buffer end, so the data block is truncated. Hence we can't
+            // parse it. Therefore we revert back by one byte (as if we never
+            // parsed it).
+            return (offset - 1);
         }
 
         uint8_t offset_end = offset + data_len;
@@ -531,7 +541,7 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
             if (offset + 1 >= buf.size()) {
                 // opt_type must be cast to integer so as it is not treated as
                 // unsigned char value (a number is presented in error message).
-                isc_throw(OutOfRange, "Attempt to parse truncated option "
+                isc_throw(OutOfRange, "Attempt to parse truncated vendor option "
                           << static_cast<int>(opt_type));
             }
 
@@ -551,7 +561,7 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
                 // to get one option definition with the particular code. If more are
                 // returned we report an error.
                 const OptionDefContainerTypeRange& range = idx->equal_range(opt_type);
-            // Get the number of returned option definitions for the option code.
+                // Get the number of returned option definitions for the option code.
                 size_t num_defs = distance(range.first, range.second);
 
                 if (num_defs > 1) {
diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h
index 90d37baf48..05247f70dd 100644
--- a/src/lib/dhcp/pkt.h
+++ b/src/lib/dhcp/pkt.h
@@ -541,17 +541,6 @@ protected:
     /// behavior must be taken into consideration before making
     /// changes to this member such as access scope restriction or
     /// data format change etc.
-    ///
-    /// @warning This public member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc. This field is also public, because
-    /// it may be modified by callouts (which are written in C++ now,
-    /// but we expect to also have them in Python, so any accesibility
-    /// methods would overly complicate things here and degrade
-    /// performance).
     isc::util::OutputBuffer buffer_out_;
 
     /// packet timestamp