Remove reserved field from ALTSVC frame

szy1210 · Jun 7, 2014 · d4602a0 · d4602a0
1 parent 3daeadc
commit d4602a0
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 36 deletions.
diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c
@@ -776,10 +776,6 @@ int nghttp2_frame_pack_altsvc(nghttp2_bufs *bufs, nghttp2_altsvc *frame)
   nghttp2_put_uint16be(buf->last, frame->port);
   buf->last += 2;
 
-  /* Reserved */
-  buf->last[0] = 0;
-  ++buf->last;
-
   buf->last[0] = frame->protocol_id_len;
   ++buf->last;
 
@@ -828,15 +824,12 @@ int nghttp2_frame_unpack_altsvc_payload(nghttp2_altsvc *frame,
   frame->port = nghttp2_get_uint16(payload);
   payload += 2;
 
-  /* Skip Reserved */
-  ++payload;
-
   frame->protocol_id_len = *payload;
 
   nghttp2_buf_wrap_init(&buf, var_gift_payload, var_gift_payloadlen);
   buf.last += var_gift_payloadlen;
 
-  /* 1 for HOST_LEN */
+  /* 1 for Host-Len */
   if(nghttp2_buf_len(&buf) < 1 + (ssize_t)frame->protocol_id_len) {
     return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }

diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h
@@ -63,12 +63,12 @@
 #define NGHTTP2_PRIORITY_SPECLEN 5
 
 /* Length of fixed part in ALTSVC frame, that is the sum of fields of
-   Max-Age, Port, Reserved and PID_LEN. */
-#define NGHTTP2_ALTSVC_FIXED_PARTLEN 8
+   Max-Age, Port and Proto-Len. */
+#define NGHTTP2_ALTSVC_FIXED_PARTLEN 7
 
 /* Minimum length of ALTSVC frame.  NGHTTP2_ALTSVC_FIXED_PARTLEN +
-   HOST_LEN. */
-#define NGHTTP2_ALTSVC_MINLEN 9
+   Host-Len. */
+#define NGHTTP2_ALTSVC_MINLEN 8
 
 
 /* Category of frames. */

diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c
@@ -472,13 +472,13 @@ void test_nghttp2_frame_pack_altsvc(void)
 
   CU_ASSERT(0 == rv);
 
-  /* 1 for HOST_LEN */
-  CU_ASSERT((ssize_t)(NGHTTP2_FRAME_HDLEN + 8 + 1 + datalen) ==
+  CU_ASSERT((ssize_t)(NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_MINLEN + datalen) ==
             nghttp2_bufs_len(&bufs));
 
   CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs));
 
-  check_frame_header(8 + 1 + datalen, NGHTTP2_ALTSVC, NGHTTP2_FLAG_NONE,
+  check_frame_header(NGHTTP2_ALTSVC_MINLEN + datalen,
+                     NGHTTP2_ALTSVC, NGHTTP2_FLAG_NONE,
                      1000000007, &oframe.hd);
   CU_ASSERT(1u << 31 == oframe.max_age);
   CU_ASSERT(4000 == oframe.port);
@@ -503,59 +503,60 @@ void test_nghttp2_frame_pack_altsvc(void)
 
   /* Check no origin case */
 
-  payloadlen = 8 + protocol_id_len + 1 + host_len;
+  payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len + host_len;
   nghttp2_put_uint16be(buf->pos, payloadlen);
 
   CU_ASSERT(0 ==
             nghttp2_frame_unpack_altsvc_payload
             (&oframe,
              buf->pos + NGHTTP2_FRAME_HDLEN,
-             8,
-             buf->pos + NGHTTP2_FRAME_HDLEN + 8,
-             payloadlen - 8));
+             NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             buf->pos + NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             payloadlen - NGHTTP2_ALTSVC_FIXED_PARTLEN));
 
   CU_ASSERT(host_len == oframe.host_len);
   CU_ASSERT(0 == oframe.origin_len);
 
   /* Check insufficient payload length for host */
-  payloadlen = 8 + protocol_id_len + 1 + host_len - 1;
+  payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len + host_len - 1;
   nghttp2_put_uint16be(buf->pos, payloadlen);
 
   CU_ASSERT(NGHTTP2_ERR_FRAME_SIZE_ERROR ==
             nghttp2_frame_unpack_altsvc_payload
             (&oframe,
              buf->pos + NGHTTP2_FRAME_HDLEN,
-             8,
-             buf->pos + NGHTTP2_FRAME_HDLEN + 8,
-             payloadlen - 8));
+             NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             buf->pos + NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             payloadlen - NGHTTP2_ALTSVC_FIXED_PARTLEN));
 
   /* Check no host case */
-  payloadlen = 8 + protocol_id_len + 1;
+  payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len;
   nghttp2_put_uint16be(buf->pos, payloadlen);
-  buf->pos[NGHTTP2_FRAME_HDLEN + 8 + protocol_id_len] = 0;
+  buf->pos[NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN
+           + protocol_id_len] = 0;
 
   CU_ASSERT(0 ==
             nghttp2_frame_unpack_altsvc_payload
             (&oframe,
              buf->pos + NGHTTP2_FRAME_HDLEN,
-             8,
-             buf->pos + NGHTTP2_FRAME_HDLEN + 8,
-             payloadlen - 8));
+             NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             buf->pos + NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             payloadlen - NGHTTP2_ALTSVC_FIXED_PARTLEN));
 
   CU_ASSERT(0 == oframe.host_len);
   CU_ASSERT(0 == oframe.origin_len);
 
-  /* Check missing HOST_LEN */
-  payloadlen = 8 + protocol_id_len;
+  /* Check missing Host-Len */
+  payloadlen = NGHTTP2_ALTSVC_FIXED_PARTLEN + protocol_id_len;
   nghttp2_put_uint16be(buf->pos, payloadlen);
 
   CU_ASSERT(NGHTTP2_ERR_FRAME_SIZE_ERROR ==
             nghttp2_frame_unpack_altsvc_payload
             (&oframe,
              buf->pos + NGHTTP2_FRAME_HDLEN,
-             8,
-             buf->pos + NGHTTP2_FRAME_HDLEN + 8,
-             payloadlen - 8));
+             NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             buf->pos + NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN,
+             payloadlen - NGHTTP2_ALTSVC_FIXED_PARTLEN));
 
   nghttp2_bufs_free(&bufs);
 }

diff --git a/tests/nghttp2_test_helper.c b/tests/nghttp2_test_helper.c
@@ -87,12 +87,13 @@ int unpack_frame(nghttp2_frame *frame, const uint8_t *in, size_t len)
       (&frame->window_update, payload, payloadlen);
     break;
   case NGHTTP2_ALTSVC:
-    gift_payloadlen = payloadlen - 8;
+    gift_payloadlen = payloadlen - NGHTTP2_ALTSVC_FIXED_PARTLEN;
     gift_payload = malloc(gift_payloadlen);
 
-    memcpy(gift_payload, payload + 8, gift_payloadlen);
+    memcpy(gift_payload, payload + NGHTTP2_ALTSVC_FIXED_PARTLEN,
+           gift_payloadlen);
 
-    payloadlen -= 8;
+    payloadlen -= NGHTTP2_ALTSVC_FIXED_PARTLEN;
 
     rv = nghttp2_frame_unpack_altsvc_payload(&frame->altsvc,
                                              payload, payloadlen,