automatically coalesce values across multiple options

diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c
index 9b12ba6b33a54a21263e74129f0da849c16cbd2b..ff850eecdccafce633af5b0d3f58612f7a16770d 100644 (file)
--- a/src/protocols/dhcpv4/decode.c
+++ b/src/protocols/dhcpv4/decode.c
@@ -504,12 +504,14 @@ next:
  * @param[in] decode_ctx       Unused.
  */
 ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
-                               uint8_t const *data, size_t data_len, UNUSED void *decode_ctx)
+                               uint8_t const *data, size_t data_len, void *decode_ctx)
 {
-       ssize_t                 ret;
-       uint8_t const           *p = data;
+       ssize_t                 slen;
+       uint8_t const           *p = data, *end = data + data_len;
+       uint8_t const           *next;
        fr_dict_attr_t const    *da;
        fr_dict_attr_t const    *parent;
+       fr_dhcpv4_ctx_t         *packet_ctx = decode_ctx;
 
        FR_PROTO_TRACE("%s called to parse %zu byte(s)", __FUNCTION__, data_len);
 
@@ -548,6 +550,42 @@ ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        if (da->type == FR_TYPE_VSA) return decode_vsa(ctx, out, da, data + 2, data[1]);
 
+       /*
+        *      Check for multiple options of the same type, and concatenate their values together.
+        */
+       next = data + 2 + data[1];
+       if ((data[1] > 0) && (next < end) && (next[0] == data[0])) {
+               uint8_t *q;
+
+               if (!packet_ctx->buffer) {
+                       packet_ctx->buffer = talloc_array(packet_ctx, uint8_t, data_len);
+                       if (!packet_ctx->buffer) return -1;
+
+               } else if (talloc_array_length(packet_ctx->buffer) < data_len) {
+                       /*
+                        *      We're called in a loop from fr_dhcpv4_decode(), with the full packet, so the
+                        *      needed size should only go down as we decode the packet.
+                        */
+                       return -1;
+               }
+               q = packet_ctx->buffer;
+
+               for (next = data; next < end; next += 2 + next[1]) {
+                       if (next[0] != data[0]) break;
+                       memcpy(q, next + 2, next[1]);
+                       q += next[1];
+               }
+
+               slen = decode_value(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer);
+               if (slen <= 0) return slen;
+
+               /*
+                *      The actual amount of data we decoded, including the various headers.
+                */
+               slen = next - data;
+               goto done;
+       }
+
        /*
         *      @todo - RFC 2131 Section 4.1 says:
         *
@@ -563,14 +601,17 @@ ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      mashed into the temporary buffer.  Then, that buffer
         *      gets used for value decoding.
         */
-       ret = decode_value(ctx, out, da, data + 2, data[1]);
-       if (ret < 0) {
+       slen = decode_value(ctx, out, da, data + 2, data[1]);
+       if (slen < 0) {
                fr_dict_unknown_free(&da);
-               return ret;
+               return slen;
        }
-       ret += 2; /* For header */
-       FR_PROTO_TRACE("decoding option complete, returning %zu byte(s)", ret);
-       return ret;
+
+       slen += 2; /* For header */
+
+done:
+       FR_PROTO_TRACE("decoding option complete, returning %zu byte(s)", slen);
+       return slen;
 }
 
 static int _decode_test_ctx(UNUSED fr_dhcpv4_ctx_t *test_ctx)

diff --git a/src/tests/unit/protocols/dhcpv4/base.txt b/src/tests/unit/protocols/dhcpv4/base.txt
index 1ac736d49f6572a436572b9524f03bd976c0083e..79acbc62853e7bfd350f0b11606c0f9f71e224bb 100644 (file)
--- a/src/tests/unit/protocols/dhcpv4/base.txt
+++ b/src/tests/unit/protocols/dhcpv4/base.txt
@@ -75,8 +75,6 @@ match 0e ff 31 32 33 34 35 36 37 38 39 30 31 32 33 34 35 36 37 38 39 30 31 32 33
 encode-pair Domain-Name = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxy"
 match 0f ff 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 0f 2d 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 79
 
-#
-#  Note that this is wrong!  RFC 2131 Section 4.1 says:
 #
 #   The client concatenates the values of multiple instances of the
 #   same option into a single parameter list for configuration.
@@ -84,7 +82,7 @@ match 0f ff 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78
 #  Presumably this means that the server does the same thing, too.
 #
 decode-pair -
-match Domain-Name = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", Domain-Name = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxy"
+match Domain-Name = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxy"
 
 #
 #  3 isn't used for anything, and should just be treated as an unknown attribute