This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/quic-latest by this push:
     new 98b258e  Prohibit duplicate transport parameters
98b258e is described below

commit 98b258efe2733145a8fe3985b62b7b7dda4d19a1
Author: Masakazu Kitajo <mas...@apache.org>
AuthorDate: Fri Dec 8 16:15:59 2017 +0900

    Prohibit duplicate transport parameters
---
 iocore/net/QUICNetVConnection.cc                   |   4 +-
 iocore/net/quic/QUICHandshake.cc                   |  21 +-
 iocore/net/quic/QUICStreamManager.cc               |  23 +-
 iocore/net/quic/QUICTransportParameters.cc         | 241 ++++++++++----------
 iocore/net/quic/QUICTransportParameters.h          |  59 ++---
 iocore/net/quic/test/test_QUICStreamManager.cc     |   6 +-
 .../net/quic/test/test_QUICTransportParameters.cc  | 243 ++++++++++++---------
 7 files changed, 311 insertions(+), 286 deletions(-)

diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 2d5e32d..3b1843a 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -863,10 +863,10 @@ QUICNetVConnection::_init_flow_control_params(const 
std::shared_ptr<const QUICTr
   uint32_t local_initial_max_data  = 0;
   uint32_t remote_initial_max_data = 0;
   if (local_tp) {
-    local_initial_max_data = local_tp->initial_max_data();
+    local_initial_max_data = 
local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_DATA);
   }
   if (remote_tp) {
-    remote_initial_max_data = remote_tp->initial_max_data();
+    remote_initial_max_data = 
remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_DATA);
   }
 
   this->_local_flow_controller->forward_limit(local_initial_max_data * 1024);
diff --git a/iocore/net/quic/QUICHandshake.cc b/iocore/net/quic/QUICHandshake.cc
index 3637feb..32f727f 100644
--- a/iocore/net/quic/QUICHandshake.cc
+++ b/iocore/net/quic/QUICHandshake.cc
@@ -284,22 +284,11 @@ QUICHandshake::_load_local_transport_parameters()
   // MUSTs
   QUICTransportParametersInEncryptedExtensions *tp = new 
QUICTransportParametersInEncryptedExtensions();
 
-  tp->add(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA,
-          std::unique_ptr<QUICTransportParameterValue>(
-            new QUICTransportParameterValue(params->initial_max_stream_data(), 
sizeof(params->initial_max_stream_data()))));
-
-  tp->add(QUICTransportParameterId::INITIAL_MAX_DATA, 
std::unique_ptr<QUICTransportParameterValue>(new QUICTransportParameterValue(
-                                                        
params->initial_max_data(), sizeof(params->initial_max_data()))));
-
-  tp->add(QUICTransportParameterId::INITIAL_MAX_STREAM_ID,
-          std::unique_ptr<QUICTransportParameterValue>(
-            new QUICTransportParameterValue(params->initial_max_stream_id(), 
sizeof(params->initial_max_stream_id()))));
-
-  tp->add(QUICTransportParameterId::IDLE_TIMEOUT, 
std::unique_ptr<QUICTransportParameterValue>(new QUICTransportParameterValue(
-                                                    
params->no_activity_timeout_in(), sizeof(uint16_t))));
-
-  tp->add(QUICTransportParameterId::STATELESS_RETRY_TOKEN,
-          std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(this->_token.buf(), 16)));
+  tp->set(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
params->initial_max_stream_data());
+  tp->set(QUICTransportParameterId::INITIAL_MAX_DATA, 
params->initial_max_data());
+  tp->set(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, 
params->initial_max_stream_id());
+  tp->set(QUICTransportParameterId::IDLE_TIMEOUT, 
static_cast<uint16_t>(params->no_activity_timeout_in()));
+  tp->set(QUICTransportParameterId::STATELESS_RETRY_TOKEN, this->_token.buf(), 
16);
 
   tp->add_version(QUIC_SUPPORTED_VERSIONS[0]);
   // MAYs
diff --git a/iocore/net/quic/QUICStreamManager.cc 
b/iocore/net/quic/QUICStreamManager.cc
index b5c10eb..3b6ac32 100644
--- a/iocore/net/quic/QUICStreamManager.cc
+++ b/iocore/net/quic/QUICStreamManager.cc
@@ -57,23 +57,19 @@ QUICStreamManager::init_flow_control_params(const 
std::shared_ptr<const QUICTran
     uint32_t local_initial_max_stream_data  = 0;
     uint32_t remote_initial_max_stream_data = 0;
     if (this->_local_tp) {
-      local_initial_max_stream_data = local_tp->initial_max_stream_data();
+      local_initial_max_stream_data = 
local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
     }
     if (this->_remote_tp) {
-      remote_initial_max_stream_data = remote_tp->initial_max_stream_data();
+      remote_initial_max_stream_data = 
remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
     }
     stream->init_flow_control_params(local_initial_max_stream_data, 
remote_initial_max_stream_data);
   }
 
-  uint16_t len;
-  const uint8_t *tp_value;
   if (this->_local_tp) {
-    tp_value                       = 
this->_local_tp->get(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, len);
-    this->_local_maximum_stream_id = QUICTypeUtil::read_QUICStreamId(tp_value, 
len);
+    this->_local_maximum_stream_id = 
this->_local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_ID);
   }
   if (this->_remote_tp) {
-    tp_value                        = 
this->_remote_tp->get(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, len);
-    this->_remote_maximum_stream_id = 
QUICTypeUtil::read_QUICStreamId(tp_value, len);
+    this->_remote_maximum_stream_id = 
this->_remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_ID);
   }
 }
 
@@ -208,14 +204,13 @@ QUICStreamManager::_find_or_create_stream(QUICStreamId 
stream_id)
     stream = new (THREAD_ALLOC(quicStreamAllocator, this_ethread())) 
QUICStream();
     if (stream_id == STREAM_ID_FOR_HANDSHAKE) {
       // XXX rece/send max_stream_data are going to be set by 
init_flow_control_params()
-      stream->init(this->_tx, this->_connection_id, stream_id, 
this->_local_tp->initial_max_stream_data());
+      stream->init(this->_tx, this->_connection_id, stream_id,
+                   
this->_local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA));
     } else {
-      const QUICTransportParameters &local_tp  = *this->_local_tp;
-      const QUICTransportParameters &remote_tp = *this->_remote_tp;
-
       // TODO: check local_tp and remote_tp is initialized
-      stream->init(this->_tx, this->_connection_id, stream_id, 
local_tp.initial_max_stream_data(),
-                   remote_tp.initial_max_stream_data());
+      stream->init(this->_tx, this->_connection_id, stream_id,
+                   
this->_local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA),
+                   
this->_remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA));
     }
 
     stream->start();
diff --git a/iocore/net/quic/QUICTransportParameters.cc 
b/iocore/net/quic/QUICTransportParameters.cc
index f8dac04..72da3ad 100644
--- a/iocore/net/quic/QUICTransportParameters.cc
+++ b/iocore/net/quic/QUICTransportParameters.cc
@@ -31,119 +31,146 @@
 
 static constexpr int TRANSPORT_PARAMETERS_MAXIMUM_SIZE = 65535;
 
-//
-// QUICTransportParameterValue
-//
-QUICTransportParameterValue::QUICTransportParameterValue(ats_unique_buf d, 
uint16_t l) : _data(std::move(d)), _len(l){};
-
-QUICTransportParameterValue::QUICTransportParameterValue(const uint8_t 
*raw_data, uint16_t l)
+QUICTransportParameters::Value::Value(const uint8_t *data, uint16_t len) : 
_len(len)
 {
-  this->_data = ats_unique_malloc(l);
-  this->_len  = l;
-  memcpy(this->_data.get(), raw_data, l);
+  this->_data = static_cast<uint8_t *>(ats_malloc(len));
+  memcpy(this->_data, data, len);
 }
 
-QUICTransportParameterValue::QUICTransportParameterValue(uint64_t raw_data, 
uint16_t l)
+QUICTransportParameters::Value::~Value()
 {
-  this->_data = ats_unique_malloc(l);
-  size_t len  = 0;
-  QUICTypeUtil::write_uint_as_nbytes(raw_data, l, this->_data.get(), &len);
-  this->_len = len;
-};
+  ats_free(this->_data);
+  this->_data = nullptr;
+}
 
-QUICTransportParameterValue::QUICTransportParameterValue(const uint64_t 
raw_data[2], uint16_t l)
+bool
+QUICTransportParameters::is_valid() const
 {
-  this->_data = ats_unique_malloc(l);
-  size_t len  = 0;
-  if (l > 8) {
-    QUICTypeUtil::write_uint_as_nbytes(raw_data[0], 8, this->_data.get(), 
&len);
-    this->_len += len;
-    QUICTypeUtil::write_uint_as_nbytes(raw_data[1], l - 8, this->_data.get() + 
8, &len);
-    this->_len += len;
-  } else {
-    QUICTypeUtil::write_uint_as_nbytes(raw_data[0], l, this->_data.get(), 
&len);
-    this->_len += len;
-  }
+  return this->_valid;
 }
 
 const uint8_t *
-QUICTransportParameterValue::data() const
+QUICTransportParameters::Value::data() const
 {
-  return this->_data.get();
+  return this->_data;
 }
 
 uint16_t
-QUICTransportParameterValue::len() const
+QUICTransportParameters::Value::len() const
 {
   return this->_len;
 }
 
-//
-// QUICTransportParameters
-//
-
-QUICTransportParameters::QUICTransportParameters(const uint8_t *buf, size_t 
len)
+QUICTransportParameters::~QUICTransportParameters()
 {
-  this->_buf = ats_unique_malloc(len);
-  memcpy(this->_buf.get(), buf, len);
+  for (auto p : this->_parameters) {
+    delete p.second;
+  }
 }
 
-const uint8_t *
-QUICTransportParameters::get(QUICTransportParameterId tpid, uint16_t &len) 
const
+void
+QUICTransportParameters::_load(const uint8_t *buf, size_t len)
 {
-  if (this->_buf) {
-    const uint8_t *p = this->_buf.get() + this->_parameters_offset();
+  bool has_error   = false;
+  const uint8_t *p = buf + this->_parameters_offset(buf);
 
-    uint16_t n = (p[0] << 8) + p[1];
-    p += 2;
-    while (n > 0) {
-      uint16_t _id = (p[0] << 8) + p[1];
+  // Read size of parameters field
+  uint16_t nbytes = (p[0] << 8) + p[1];
+  p += 2;
+
+  // Read parameters
+  const uint8_t *end = p + nbytes;
+  while (p < end) {
+    // Read ID
+    uint16_t id = 0;
+    if (end - p >= 2) {
+      id = (p[0] << 8) + p[1];
       p += 2;
-      n -= 2;
-      uint16_t _value_len = (p[0] << 8) + p[1];
+    } else {
+      has_error = true;
+      break;
+    }
+
+    // Check duplication
+    // An endpoint MUST treat receipt of duplicate transport parameters as a 
connection error of type TRANSPORT_PARAMETER_ERROR
+    if (this->_parameters.find(id) != this->_parameters.end()) {
+      has_error = true;
+      break;
+    }
+
+    // Read length of value
+    uint16_t len = 0;
+    if (end - p >= 2) {
+      len = (p[0] << 8) + p[1];
       p += 2;
-      n -= 2;
-      if (tpid == _id) {
-        len = _value_len;
-        return p;
-      }
-      p += _value_len;
-      n -= _value_len;
+    } else {
+      has_error = true;
+      break;
     }
-  } else {
-    auto p = 
this->_parameters.find(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
-    if (p != this->_parameters.end()) {
-      len = p->second->len();
-      return p->second->data();
+
+    // Store parameter
+    if (end - p >= len) {
+      this->_parameters.insert(std::make_pair(id, new Value(p, len)));
+      p += len;
+    } else {
+      has_error = true;
+      break;
     }
   }
 
+  this->_valid = !has_error;
+}
+
+const uint8_t *
+QUICTransportParameters::getAsBytes(QUICTransportParameterId tpid, uint16_t 
&len) const
+{
+  auto p = this->_parameters.find(tpid);
+  if (p != this->_parameters.end()) {
+    len = p->second->len();
+    return p->second->data();
+  }
+
   len = 0;
   return nullptr;
 }
 
 uint32_t
-QUICTransportParameters::initial_max_stream_data() const
+QUICTransportParameters::getAsUInt32(QUICTransportParameterId tpid) const
 {
-  uint16_t len        = 0;
-  const uint8_t *data = 
this->get(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, len);
-
-  return static_cast<uint32_t>(QUICTypeUtil::read_nbytes_as_uint(data, len));
+  uint16_t len         = 0;
+  const uint8_t *value = this->getAsBytes(tpid, len);
+  if (value) {
+    return QUICTypeUtil::read_nbytes_as_uint(value, 4);
+  } else {
+    return 0;
+  }
 }
 
-uint32_t
-QUICTransportParameters::initial_max_data() const
+void
+QUICTransportParameters::set(QUICTransportParameterId id, const uint8_t 
*value, uint16_t value_len)
 {
-  uint16_t len        = 0;
-  const uint8_t *data = this->get(QUICTransportParameterId::INITIAL_MAX_DATA, 
len);
+  if (this->_parameters.find(id) != this->_parameters.end()) {
+    this->_parameters.erase(id);
+  }
+  this->_parameters.insert(std::make_pair(id, new Value(value, value_len)));
+}
 
-  return static_cast<uint32_t>(QUICTypeUtil::read_nbytes_as_uint(data, len));
+void
+QUICTransportParameters::set(QUICTransportParameterId id, uint16_t value)
+{
+  uint8_t v[2];
+  size_t n;
+  QUICTypeUtil::write_uint_as_nbytes(value, 2, v, &n);
+  this->set(id, v, 2);
 }
 
 void
-QUICTransportParameters::add(QUICTransportParameterId id, 
std::unique_ptr<QUICTransportParameterValue> value)
+QUICTransportParameters::set(QUICTransportParameterId id, uint32_t value)
 {
-  this->_parameters.insert(std::pair<QUICTransportParameterId, 
std::unique_ptr<QUICTransportParameterValue>>(id, std::move(value)));
+  uint8_t v[4];
+  size_t n;
+  QUICTypeUtil::write_uint_as_nbytes(value, 4, v, &n);
+  this->set(id, v, 4);
 }
 
 void
@@ -164,12 +191,11 @@ QUICTransportParameters::store(uint8_t *buf, uint16_t 
*len) const
     p[0] = (it.first & 0xff00) >> 8;
     p[1] = it.first & 0xff;
     p += 2;
-    const QUICTransportParameterValue *value = it.second.get();
-    p[0]                                     = (value->len() & 0xff00) >> 8;
-    p[1]                                     = value->len() & 0xff;
+    p[0] = (it.second->len() & 0xff00) >> 8;
+    p[1] = it.second->len() & 0xff;
     p += 2;
-    memcpy(p, value->data(), value->len());
-    p += value->len();
+    memcpy(p, it.second->data(), it.second->len());
+    p += it.second->len();
   }
 
   ptrdiff_t n = p - parameters_size - sizeof(uint16_t);
@@ -185,29 +211,22 @@ QUICTransportParameters::store(uint8_t *buf, uint16_t 
*len) const
 //
 
 
QUICTransportParametersInClientHello::QUICTransportParametersInClientHello(const
 uint8_t *buf, size_t len)
-  : QUICTransportParameters(buf, len)
 {
+  this->_load(buf, len);
+  this->_negotiated_version = QUICTypeUtil::read_QUICVersion(buf);
+  this->_initial_version    = QUICTypeUtil::read_QUICVersion(buf + 
sizeof(QUICVersion));
+
   // Print all parameters
-  const uint8_t *p = this->_buf.get() + this->_parameters_offset();
-  uint16_t n       = (p[0] << 8) + p[1];
-  p += 2;
-  while (n > 0) {
-    uint16_t _id = (p[0] << 8) + p[1];
-    p += 2;
-    n -= 2;
-    uint16_t _value_len = (p[0] << 8) + p[1];
-    p += 2;
-    n -= 2;
-    if (_value_len == 0) {
-      Debug("quic_handsahke", "%s: (no value)", 
QUICDebugNames::transport_parameter_id(_id));
-    } else if (_value_len <= 8) {
-      Debug("quic_handsahke", "%s: 0x%" PRIx64 " (%" PRIu64 ")", 
QUICDebugNames::transport_parameter_id(_id),
-            QUICTypeUtil::read_nbytes_as_uint(p, _value_len), 
QUICTypeUtil::read_nbytes_as_uint(p, _value_len));
+  for (auto &p : this->_parameters) {
+    if (p.second->len() == 0) {
+      Debug("quic_handsahke", "%s: (no value)", 
QUICDebugNames::transport_parameter_id(p.first));
+    } else if (p.second->len() <= 8) {
+      Debug("quic_handsahke", "%s: 0x%" PRIx64 " (%" PRIu64 ")", 
QUICDebugNames::transport_parameter_id(p.first),
+            QUICTypeUtil::read_nbytes_as_uint(p.second->data(), 
p.second->len()),
+            QUICTypeUtil::read_nbytes_as_uint(p.second->data(), 
p.second->len()));
     } else {
-      Debug("quic_handsahke", "%s: (long data)", 
QUICDebugNames::transport_parameter_id(_id));
+      Debug("quic_handsahke", "%s: (long data)", 
QUICDebugNames::transport_parameter_id(p.first));
     }
-    p += _value_len;
-    n -= _value_len;
   }
 }
 
@@ -224,7 +243,7 @@ QUICTransportParametersInClientHello::_store(uint8_t *buf, 
uint16_t *len) const
 }
 
 std::ptrdiff_t
-QUICTransportParametersInClientHello::_parameters_offset() const
+QUICTransportParametersInClientHello::_parameters_offset(const uint8_t *) const
 {
   return 8; // sizeof(QUICVersion) + sizeof(QUICVersion)
 }
@@ -232,27 +251,24 @@ 
QUICTransportParametersInClientHello::_parameters_offset() const
 QUICVersion
 QUICTransportParametersInClientHello::negotiated_version() const
 {
-  if (this->_buf) {
-    return QUICTypeUtil::read_QUICVersion(this->_buf.get());
-  } else {
-    return this->_negotiated_version;
-  }
+  return this->_negotiated_version;
 }
 
 QUICVersion
 QUICTransportParametersInClientHello::initial_version() const
 {
-  if (this->_buf) {
-    return QUICTypeUtil::read_QUICVersion(this->_buf.get() + 
sizeof(QUICVersion));
-  } else {
-    return this->_initial_version;
-  }
+  return this->_initial_version;
 }
 
 //
 // QUICTransportParametersInEncryptedExtensions
 //
 
+QUICTransportParametersInEncryptedExtensions::QUICTransportParametersInEncryptedExtensions(const
 uint8_t *buf, size_t len)
+{
+  this->_load(buf, len);
+}
+
 void
 QUICTransportParametersInEncryptedExtensions::_store(uint8_t *buf, uint16_t 
*len) const
 {
@@ -268,14 +284,6 @@ 
QUICTransportParametersInEncryptedExtensions::_store(uint8_t *buf, uint16_t *len
   *len = p - buf;
 }
 
-const uint8_t *
-QUICTransportParametersInEncryptedExtensions::supported_versions_len(uint16_t 
*n) const
-{
-  uint8_t *b = this->_buf.get();
-  *n         = b[0];
-  return b + 1;
-}
-
 void
 QUICTransportParametersInEncryptedExtensions::add_version(QUICVersion version)
 {
@@ -283,10 +291,9 @@ 
QUICTransportParametersInEncryptedExtensions::add_version(QUICVersion version)
 }
 
 std::ptrdiff_t
-QUICTransportParametersInEncryptedExtensions::_parameters_offset() const
+QUICTransportParametersInEncryptedExtensions::_parameters_offset(const uint8_t 
*buf) const
 {
-  const uint8_t *b = this->_buf.get();
-  return sizeof(uint8_t) + b[0];
+  return 1 + buf[0];
 }
 
 //
diff --git a/iocore/net/quic/QUICTransportParameters.h 
b/iocore/net/quic/QUICTransportParameters.h
index 12c4892..c8fabe0 100644
--- a/iocore/net/quic/QUICTransportParameters.h
+++ b/iocore/net/quic/QUICTransportParameters.h
@@ -24,7 +24,7 @@
 #pragma once
 
 #include <map>
-// #include "ts/Map.h"
+#include <ts/ink_memory.h>
 
 #include <openssl/ssl.h>
 #include "QUICTypes.h"
@@ -64,39 +64,45 @@ private:
   uint16_t _id = 0;
 };
 
-class QUICTransportParameterValue
+class QUICTransportParameters
 {
 public:
-  QUICTransportParameterValue(ats_unique_buf d, uint16_t l);
-  QUICTransportParameterValue(const uint8_t *raw_data, uint16_t l);
-  QUICTransportParameterValue(uint64_t raw_data, uint16_t l);
-  QUICTransportParameterValue(const uint64_t raw_data[2], uint16_t l);
+  QUICTransportParameters(const uint8_t *buf, size_t len);
+  ~QUICTransportParameters();
 
-  const uint8_t *data() const;
-  uint16_t len() const;
+  bool is_valid() const;
 
-private:
-  ats_unique_buf _data = {nullptr, [](void *p) { ats_free(p); }};
-  uint16_t _len        = 0;
-};
+  const uint8_t *getAsBytes(QUICTransportParameterId id, uint16_t &len) const;
+  uint32_t getAsUInt32(QUICTransportParameterId id) const;
+
+  void set(QUICTransportParameterId id, const uint8_t *value, uint16_t 
value_len);
+  void set(QUICTransportParameterId id, uint16_t value);
+  void set(QUICTransportParameterId id, uint32_t value);
 
-class QUICTransportParameters
-{
-public:
-  QUICTransportParameters(const uint8_t *buf, size_t len);
-  const uint8_t *get(QUICTransportParameterId id, uint16_t &len) const;
-  uint32_t initial_max_stream_data() const;
-  uint32_t initial_max_data() const;
-  void add(QUICTransportParameterId id, 
std::unique_ptr<QUICTransportParameterValue> value);
   void store(uint8_t *buf, uint16_t *len) const;
 
 protected:
+  class Value
+  {
+  public:
+    Value(const uint8_t *data, uint16_t len);
+    ~Value();
+    const uint8_t *data() const;
+    uint16_t len() const;
+
+  private:
+    uint8_t *_data = nullptr;
+    uint16_t _len  = 0;
+  };
+
   QUICTransportParameters(){};
-  virtual std::ptrdiff_t _parameters_offset() const = 0;
+  void _load(const uint8_t *buf, size_t len);
+  bool _valid = false;
+
+  virtual std::ptrdiff_t _parameters_offset(const uint8_t *buf) const = 0;
   virtual void _store(uint8_t *buf, uint16_t *len) const = 0;
-  ats_unique_buf _buf = {nullptr, [](void *p) { ats_free(p); }};
 
-  std::map<QUICTransportParameterId, 
std::unique_ptr<QUICTransportParameterValue>> _parameters;
+  std::map<QUICTransportParameterId, Value *> _parameters;
 };
 
 class QUICTransportParametersInClientHello : public QUICTransportParameters
@@ -109,7 +115,7 @@ public:
   QUICVersion initial_version() const;
 
 protected:
-  std::ptrdiff_t _parameters_offset() const override;
+  std::ptrdiff_t _parameters_offset(const uint8_t *buf) const override;
   void _store(uint8_t *buf, uint16_t *len) const override;
 
 private:
@@ -121,12 +127,11 @@ class QUICTransportParametersInEncryptedExtensions : 
public QUICTransportParamet
 {
 public:
   QUICTransportParametersInEncryptedExtensions() : QUICTransportParameters(){};
-  QUICTransportParametersInEncryptedExtensions(const uint8_t *buf, size_t len) 
: QUICTransportParameters(buf, len){};
-  const uint8_t *supported_versions_len(uint16_t *n) const;
+  QUICTransportParametersInEncryptedExtensions(const uint8_t *buf, size_t len);
   void add_version(QUICVersion version);
 
 protected:
-  std::ptrdiff_t _parameters_offset() const override;
+  std::ptrdiff_t _parameters_offset(const uint8_t *buf) const override;
   void _store(uint8_t *buf, uint16_t *len) const override;
 
   uint8_t _n_versions        = 0;
diff --git a/iocore/net/quic/test/test_QUICStreamManager.cc 
b/iocore/net/quic/test/test_QUICStreamManager.cc
index 3a143df..cbcb41c 100644
--- a/iocore/net/quic/test/test_QUICStreamManager.cc
+++ b/iocore/net/quic/test/test_QUICStreamManager.cc
@@ -103,8 +103,7 @@ TEST_CASE("QUICStreamManager_total_offset_received", 
"[quic]")
   app_map.set_default(&mock_app);
   QUICStreamManager sm(0, &tx, &app_map);
   std::shared_ptr<QUICTransportParameters> local_tp = 
std::make_shared<QUICTransportParametersInEncryptedExtensions>();
-  local_tp->add(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA,
-                std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(4096, 4)));
+  local_tp->set(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
UINT32_C(4096));
   std::shared_ptr<QUICTransportParameters> remote_tp =
     
std::make_shared<QUICTransportParametersInClientHello>(static_cast<QUICVersion>(0),
 static_cast<QUICVersion>(0));
   sm.init_flow_control_params(local_tp, remote_tp);
@@ -137,8 +136,7 @@ TEST_CASE("QUICStreamManager_total_offset_sent", "[quic]")
   app_map.set_default(&mock_app);
   QUICStreamManager sm(0, &tx, &app_map);
   std::shared_ptr<QUICTransportParameters> local_tp = 
std::make_shared<QUICTransportParametersInEncryptedExtensions>();
-  local_tp->add(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA,
-                std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(4096, 4)));
+  local_tp->set(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
UINT32_C(4096));
   std::shared_ptr<QUICTransportParameters> remote_tp =
     
std::make_shared<QUICTransportParametersInClientHello>(static_cast<QUICVersion>(0),
 static_cast<QUICVersion>(0));
   sm.init_flow_control_params(local_tp, remote_tp);
diff --git a/iocore/net/quic/test/test_QUICTransportParameters.cc 
b/iocore/net/quic/test/test_QUICTransportParameters.cc
index b65e406..c467939 100644
--- a/iocore/net/quic/test/test_QUICTransportParameters.cc
+++ b/iocore/net/quic/test/test_QUICTransportParameters.cc
@@ -27,50 +27,72 @@
 
 TEST_CASE("QUICTransportParametersInClientHello_read", "[quic]")
 {
-  uint8_t buf[] = {
-    0x01, 0x02, 0x03, 0x04, // negotiated version
-    0x05, 0x06, 0x07, 0x08, // iinitial version
-    0x00, 0x1e,             // size of parameters
-    0x00, 0x00,             // parameter id
-    0x00, 0x04,             // length of value
-    0x11, 0x22, 0x33, 0x44, // value
-    0x00, 0x01,             // parameter id
-    0x00, 0x04,             // length of value
-    0x12, 0x34, 0x56, 0x78, // value
-    0x00, 0x02,             // parameter id
-    0x00, 0x04,             // length of value
-    0x0a, 0x0b, 0x0c, 0x0d, // value
-    0x00, 0x03,             // parameter id
-    0x00, 0x02,             // length of value
-    0xab, 0xcd,             // value
-  };
-
-  QUICTransportParametersInClientHello params_in_ch(buf, sizeof(buf));
-  CHECK(params_in_ch.negotiated_version() == 0x01020304);
-  CHECK(params_in_ch.initial_version() == 0x05060708);
-
-  uint16_t len        = 0;
-  const uint8_t *data = nullptr;
-
-  data = params_in_ch.get(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x11\x22\x33\x44", 4) == 0);
-
-  data = params_in_ch.get(QUICTransportParameterId::INITIAL_MAX_DATA, len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x12\x34\x56\x78", 4) == 0);
-
-  data = params_in_ch.get(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, 
len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x0a\x0b\x0c\x0d", 4) == 0);
-
-  data = params_in_ch.get(QUICTransportParameterId::IDLE_TIMEOUT, len);
-  CHECK(len == 2);
-  CHECK(memcmp(data, "\xab\xcd", 2) == 0);
-
-  data = params_in_ch.get(QUICTransportParameterId::MAX_PACKET_SIZE, len);
-  CHECK(len == 0);
-  CHECK(data == nullptr);
+  SECTION("OK")
+  {
+    uint8_t buf[] = {
+      0x01, 0x02, 0x03, 0x04, // negotiated version
+      0x05, 0x06, 0x07, 0x08, // iinitial version
+      0x00, 0x1e,             // size of parameters
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x11, 0x22, 0x33, 0x44, // value
+      0x00, 0x01,             // parameter id
+      0x00, 0x04,             // length of value
+      0x12, 0x34, 0x56, 0x78, // value
+      0x00, 0x02,             // parameter id
+      0x00, 0x04,             // length of value
+      0x0a, 0x0b, 0x0c, 0x0d, // value
+      0x00, 0x03,             // parameter id
+      0x00, 0x02,             // length of value
+      0xab, 0xcd,             // value
+    };
+
+    QUICTransportParametersInClientHello params_in_ch(buf, sizeof(buf));
+    CHECK(params_in_ch.is_valid());
+    CHECK(params_in_ch.negotiated_version() == 0x01020304);
+    CHECK(params_in_ch.initial_version() == 0x05060708);
+
+    uint16_t len        = 0;
+    const uint8_t *data = nullptr;
+
+    data = 
params_in_ch.getAsBytes(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x11\x22\x33\x44", 4) == 0);
+
+    data = params_in_ch.getAsBytes(QUICTransportParameterId::INITIAL_MAX_DATA, 
len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x12\x34\x56\x78", 4) == 0);
+
+    data = 
params_in_ch.getAsBytes(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x0a\x0b\x0c\x0d", 4) == 0);
+
+    data = params_in_ch.getAsBytes(QUICTransportParameterId::IDLE_TIMEOUT, 
len);
+    CHECK(len == 2);
+    CHECK(memcmp(data, "\xab\xcd", 2) == 0);
+
+    data = params_in_ch.getAsBytes(QUICTransportParameterId::MAX_PACKET_SIZE, 
len);
+    CHECK(len == 0);
+    CHECK(data == nullptr);
+  }
+
+  SECTION("Duplicate parameters")
+  {
+    uint8_t buf[] = {
+      0x01, 0x02, 0x03, 0x04, // negotiated version
+      0x05, 0x06, 0x07, 0x08, // iinitial version
+      0x00, 0x10,             // size of parameters
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x11, 0x22, 0x33, 0x44, // value
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x12, 0x34, 0x56, 0x78, // value
+    };
+
+    QUICTransportParametersInClientHello params_in_ch(buf, sizeof(buf));
+    CHECK(!params_in_ch.is_valid());
+  }
 }
 
 TEST_CASE("QUICTransportParametersInClientHello_write", "[quic]")
@@ -97,18 +119,14 @@ TEST_CASE("QUICTransportParametersInClientHello_write", 
"[quic]")
   QUICTransportParametersInClientHello params_in_ch(0x01020304, 0x05060708);
 
   uint32_t max_stream_data = 0x11223344;
-  params_in_ch.add(
-    QUICTransportParameterId::INITIAL_MAX_STREAM_DATA,
-    std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(max_stream_data, sizeof(max_stream_data))));
+  params_in_ch.set(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
max_stream_data);
 
   uint16_t max_packet_size = 0xabcd;
-  params_in_ch.add(
-    QUICTransportParameterId::MAX_PACKET_SIZE,
-    std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(max_packet_size, sizeof(max_packet_size))));
+  params_in_ch.set(QUICTransportParameterId::MAX_PACKET_SIZE, max_packet_size);
 
-  uint64_t stateless_retry_token[2] = {0x0011223344556677, 0x0011223344556677};
-  params_in_ch.add(QUICTransportParameterId::STATELESS_RETRY_TOKEN,
-                   std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(stateless_retry_token, 16)));
+  uint8_t stateless_retry_token[16] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 
0x66, 0x77,
+                                       0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 
0x66, 0x77};
+  params_in_ch.set(QUICTransportParameterId::STATELESS_RETRY_TOKEN, 
stateless_retry_token, 16);
 
   params_in_ch.store(buf, &len);
   CHECK(len == 44);
@@ -117,53 +135,70 @@ TEST_CASE("QUICTransportParametersInClientHello_write", 
"[quic]")
 
 TEST_CASE("QUICTransportParametersInEncryptedExtensions_read", "[quic]")
 {
-  uint8_t buf[] = {
-    0x04,                   // size of supported versions
-    0x01, 0x02, 0x03, 0x04, //
-    0x00, 0x1e,             // size of parameters
-    0x00, 0x00,             // parameter id
-    0x00, 0x04,             // length of value
-    0x11, 0x22, 0x33, 0x44, // value
-    0x00, 0x01,             // parameter id
-    0x00, 0x04,             // length of value
-    0x12, 0x34, 0x56, 0x78, // value
-    0x00, 0x02,             // parameter id
-    0x00, 0x04,             // length of value
-    0x0a, 0x0b, 0x0c, 0x0d, // value
-    0x00, 0x03,             // parameter id
-    0x00, 0x02,             // length of value
-    0xab, 0xcd,             // value
-  };
-
-  QUICTransportParametersInEncryptedExtensions params_in_ee(buf, sizeof(buf));
-  const uint8_t *versions;
-  uint16_t vlen;
-  versions = params_in_ee.supported_versions_len(&vlen);
-  CHECK(vlen == 4);
-  CHECK(memcmp(versions, "\x01\x02\x03\x04", 4) == 0);
-
-  uint16_t len        = 0;
-  const uint8_t *data = nullptr;
-
-  data = params_in_ee.get(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x11\x22\x33\x44", 4) == 0);
-
-  data = params_in_ee.get(QUICTransportParameterId::INITIAL_MAX_DATA, len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x12\x34\x56\x78", 4) == 0);
-
-  data = params_in_ee.get(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, 
len);
-  CHECK(len == 4);
-  CHECK(memcmp(data, "\x0a\x0b\x0c\x0d", 4) == 0);
-
-  data = params_in_ee.get(QUICTransportParameterId::IDLE_TIMEOUT, len);
-  CHECK(len == 2);
-  CHECK(memcmp(data, "\xab\xcd", 2) == 0);
-
-  data = params_in_ee.get(QUICTransportParameterId::MAX_PACKET_SIZE, len);
-  CHECK(len == 0);
-  CHECK(data == nullptr);
+  SECTION("OK")
+  {
+    uint8_t buf[] = {
+      0x04,                   // size of supported versions
+      0x01, 0x02, 0x03, 0x04, //
+      0x00, 0x1e,             // size of parameters
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x11, 0x22, 0x33, 0x44, // value
+      0x00, 0x01,             // parameter id
+      0x00, 0x04,             // length of value
+      0x12, 0x34, 0x56, 0x78, // value
+      0x00, 0x02,             // parameter id
+      0x00, 0x04,             // length of value
+      0x0a, 0x0b, 0x0c, 0x0d, // value
+      0x00, 0x03,             // parameter id
+      0x00, 0x02,             // length of value
+      0xab, 0xcd,             // value
+    };
+
+    QUICTransportParametersInEncryptedExtensions params_in_ee(buf, 
sizeof(buf));
+    CHECK(params_in_ee.is_valid());
+
+    uint16_t len        = 0;
+    const uint8_t *data = nullptr;
+
+    data = 
params_in_ee.getAsBytes(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x11\x22\x33\x44", 4) == 0);
+
+    data = params_in_ee.getAsBytes(QUICTransportParameterId::INITIAL_MAX_DATA, 
len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x12\x34\x56\x78", 4) == 0);
+
+    data = 
params_in_ee.getAsBytes(QUICTransportParameterId::INITIAL_MAX_STREAM_ID, len);
+    CHECK(len == 4);
+    CHECK(memcmp(data, "\x0a\x0b\x0c\x0d", 4) == 0);
+
+    data = params_in_ee.getAsBytes(QUICTransportParameterId::IDLE_TIMEOUT, 
len);
+    CHECK(len == 2);
+    CHECK(memcmp(data, "\xab\xcd", 2) == 0);
+
+    data = params_in_ee.getAsBytes(QUICTransportParameterId::MAX_PACKET_SIZE, 
len);
+    CHECK(len == 0);
+    CHECK(data == nullptr);
+  }
+
+  SECTION("Duplicate parameters")
+  {
+    uint8_t buf[] = {
+      0x04,                   // size of supported versions
+      0x01, 0x02, 0x03, 0x04, //
+      0x00, 0x1e,             // size of parameters
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x01, 0x02, 0x03, 0x04, // value
+      0x00, 0x00,             // parameter id
+      0x00, 0x04,             // length of value
+      0x12, 0x34, 0x56, 0x78, // value
+    };
+
+    QUICTransportParametersInEncryptedExtensions params_in_ee(buf, 
sizeof(buf));
+    CHECK(!params_in_ee.is_valid());
+  }
 }
 
 TEST_CASE("QUICTransportParametersEncryptedExtensions_write", "[quic]")
@@ -187,14 +222,10 @@ 
TEST_CASE("QUICTransportParametersEncryptedExtensions_write", "[quic]")
   QUICTransportParametersInEncryptedExtensions params_in_ee;
 
   uint32_t max_stream_data = 0x11223344;
-  params_in_ee.add(
-    QUICTransportParameterId::INITIAL_MAX_STREAM_DATA,
-    std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(max_stream_data, sizeof(max_stream_data))));
+  params_in_ee.set(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA, 
max_stream_data);
 
   uint16_t max_packet_size = 0xabcd;
-  params_in_ee.add(
-    QUICTransportParameterId::MAX_PACKET_SIZE,
-    std::unique_ptr<QUICTransportParameterValue>(new 
QUICTransportParameterValue(max_packet_size, sizeof(max_packet_size))));
+  params_in_ee.set(QUICTransportParameterId::MAX_PACKET_SIZE, max_packet_size);
 
   params_in_ee.add_version(0x01020304);
   params_in_ee.add_version(0x05060708);

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Reply via email to