Update QUIC TP format

apache · Apr 3, 2020 · 69713f5 · 69713f5
1 parent c6221cf
commit 69713f5
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 60 deletions.
diff --git a/iocore/net/quic/QUICTransportParameters.cc b/iocore/net/quic/QUICTransportParameters.cc
@@ -33,6 +33,8 @@
 
 static constexpr char tag[] = "quic_handshake";
 
+static constexpr int TRANSPORT_PARAMETERS_MAXIMUM_SIZE = 65535;
+
 static constexpr uint32_t TP_ERROR_LENGTH = 0x010000;
 static constexpr uint32_t TP_ERROR_VALUE  = 0x020000;
 // static constexpr uint32_t TP_ERROR_MUST_EXIST     = 0x030000;
@@ -80,50 +82,42 @@ QUICTransportParameters::_load(const uint8_t *buf, size_t len)
 {
   bool has_error   = false;
   const uint8_t *p = buf;
-
-  if (len < 2) {
-    this->_valid = false;
-    return;
-  }
-
-  // Read size of parameters field
-  uint16_t nbytes = (p[0] << 8) + p[1];
-  p += 2;
+  size_t l;
+  uint64_t param_id;
+  uint64_t param_len;
 
   // Read parameters
-  const uint8_t *end = p + nbytes;
-  while (p < end) {
+  while (len) {
     // Read ID
-    uint16_t id = 0;
-    if (end - p >= 2) {
-      id = (p[0] << 8) + p[1];
-      p += 2;
+    if (!QUICVariableInt::decode(param_id, l, p, len)) {
+      len -= l;
+      p += l;
     } 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()) {
+    if (this->_parameters.find(param_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;
+    if (!QUICVariableInt::decode(param_len, l, p, len)) {
+      len -= l;
+      p += l;
     } else {
       has_error = true;
       break;
     }
 
     // Store parameter
-    if (end - p >= len) {
-      this->_parameters.insert(std::make_pair(id, new Value(p, len)));
-      p += len;
+    if (len >= param_len) {
+      this->_parameters.insert(std::make_pair(param_id, new Value(p, param_len)));
+      len -= param_len;
+      p += param_len;
     } else {
       has_error = true;
       break;
@@ -254,33 +248,19 @@ void
 QUICTransportParameters::store(uint8_t *buf, uint16_t *len) const
 {
   uint8_t *p = buf;
+  size_t l;
 
-  // Write QUIC versions
-  this->_store(p, len);
-  p += *len;
-
-  // Write parameters
-  // XXX parameters_size will be written later
-  uint8_t *parameters_size = p;
-  p += sizeof(uint16_t);
-
+  *len = 0;
   for (auto &it : this->_parameters) {
     // TODO Skip non-MUST parameters that have their default values
-    p[0] = (it.first & 0xff00) >> 8;
-    p[1] = it.first & 0xff;
-    p += 2;
-    p[0] = (it.second->len() & 0xff00) >> 8;
-    p[1] = it.second->len() & 0xff;
-    p += 2;
+    QUICVariableInt::encode(p, TRANSPORT_PARAMETERS_MAXIMUM_SIZE, l, it.first);
+    p += l;
+    QUICVariableInt::encode(p, TRANSPORT_PARAMETERS_MAXIMUM_SIZE, l, it.second->len());
+    p += l;
     memcpy(p, it.second->data(), it.second->len());
     p += it.second->len();
   }
 
-  ptrdiff_t n = p - parameters_size - sizeof(uint16_t);
-
-  parameters_size[0] = (n & 0xff00) >> 8;
-  parameters_size[1] = n & 0xff;
-
   *len = (p - buf);
 }
 
@@ -329,12 +309,6 @@ QUICTransportParametersInClientHello::QUICTransportParametersInClientHello(const
   }
 }
 
-void
-QUICTransportParametersInClientHello::_store(uint8_t *buf, uint16_t *len) const
-{
-  *len = 0;
-}
-
 std::ptrdiff_t
 QUICTransportParametersInClientHello::_parameters_offset(const uint8_t *) const
 {
@@ -375,12 +349,6 @@ QUICTransportParametersInEncryptedExtensions::QUICTransportParametersInEncrypted
   }
 }
 
-void
-QUICTransportParametersInEncryptedExtensions::_store(uint8_t *buf, uint16_t *len) const
-{
-  *len = 0;
-}
-
 void
 QUICTransportParametersInEncryptedExtensions::add_version(QUICVersion version)
 {
@@ -429,8 +397,6 @@ QUICTransportParametersInEncryptedExtensions::_validate_parameters() const
 
 #ifndef OPENSSL_IS_BORINGSSL
 
-static constexpr int TRANSPORT_PARAMETERS_MAXIMUM_SIZE = 65535;
-
 //
 // QUICTransportParametersHandler
 //

diff --git a/iocore/net/quic/QUICTransportParameters.h b/iocore/net/quic/QUICTransportParameters.h
@@ -109,7 +109,6 @@ class QUICTransportParameters
 
   virtual std::ptrdiff_t _parameters_offset(const uint8_t *buf) const = 0;
   virtual int _validate_parameters() const;
-  virtual void _store(uint8_t *buf, uint16_t *len) const = 0;
   void _print() const;
 
   std::map<QUICTransportParameterId, Value *> _parameters;
@@ -124,7 +123,6 @@ class QUICTransportParametersInClientHello : public QUICTransportParameters
 protected:
   std::ptrdiff_t _parameters_offset(const uint8_t *buf) const override;
   int _validate_parameters() const override;
-  void _store(uint8_t *buf, uint16_t *len) const override;
 
 private:
 };
@@ -139,7 +137,6 @@ class QUICTransportParametersInEncryptedExtensions : public QUICTransportParamet
 protected:
   std::ptrdiff_t _parameters_offset(const uint8_t *buf) const override;
   int _validate_parameters() const override;
-  void _store(uint8_t *buf, uint16_t *len) const override;
 
   uint8_t _n_versions        = 0;
   QUICVersion _versions[256] = {};