fix inverted split for DATA frame padding (#330)

src/frame/data.rs

@@ -63,6 +63,18 @@ impl<T> Data<T> {
         }
     }
 
+    /// Returns whther the `PADDED` flag is set on this frame.
+    #[cfg(feature = "unstable")]
+    pub fn is_padded(&self) -> bool {
+        self.flags.is_padded()
+    }
+
+    /// Sets the value for the `PADDED` flag on this frame.
+    #[cfg(feature = "unstable")]
+    pub fn set_padded(&mut self) {
+        self.flags.set_padded();
+    }
+
     /// Returns a reference to this frame's payload.
     ///
     /// This does **not** include any padding that might have been originally
@@ -192,6 +204,11 @@ impl DataFlags {
     fn is_padded(&self) -> bool {
         self.0 & PADDED == PADDED
     }
+
+    #[cfg(feature = "unstable")]
+    fn set_padded(&mut self) {
+        self.0 |= PADDED
+    }
 }
 
 impl Default for DataFlags {

src/frame/util.rs

@@ -15,7 +15,8 @@ use bytes::Bytes;
 /// If the padded payload is invalid (e.g. the length of the padding is equal
 /// to the total length), returns `None`.
 pub fn strip_padding(payload: &mut Bytes) -> Result<u8, Error> {
-    if payload.len() == 0 {
+    let payload_len = payload.len();
+    if payload_len == 0 {
         // If this is the case, the frame is invalid as no padding length can be
         // extracted, even though the frame should be padded.
         return Err(Error::TooMuchPadding);
@@ -23,14 +24,14 @@ pub fn strip_padding(payload: &mut Bytes) -> Result<u8, Error> {
 
     let pad_len = payload[0] as usize;
 
-    if pad_len >= payload.len() {
+    if pad_len >= payload_len {
         // This is invalid: the padding length MUST be less than the
         // total frame size.
         return Err(Error::TooMuchPadding);
     }
 
     let _ = payload.split_to(1);
-    let _ = payload.split_off(pad_len);
+    let _ = payload.split_off(payload_len - pad_len - 1);
 
     Ok(pad_len as u8)
 }

tests/h2-support/src/frames.rs

@@ -180,6 +180,11 @@ impl From<Mock<frame::Headers>> for SendFrame {
 // Data helpers
 
 impl Mock<frame::Data> {
+    pub fn padded(mut self) -> Self {
+        self.0.set_padded();
+        self
+    }
+
     pub fn eos(mut self) -> Self {
         self.0.set_end_stream(true);
         self
@@ -190,9 +195,13 @@ impl From<Mock<frame::Data>> for SendFrame {
     fn from(src: Mock<frame::Data>) -> Self {
         let id = src.0.stream_id();
         let eos = src.0.is_end_stream();
+        let is_padded = src.0.is_padded();
         let payload = src.0.into_payload();
         let mut frame = frame::Data::new(id, payload.into_buf());
         frame.set_end_stream(eos);
+        if is_padded {
+            frame.set_padded();
+        }
         Frame::Data(frame)
     }
 }

tests/h2-tests/tests/codec_read.rs

@@ -72,16 +72,16 @@ fn read_data_end_stream() {
 fn read_data_padding() {
     let mut codec = raw_codec! {
         read => [
-            0, 0, 11, 0, 0x8, 0, 0, 0, 1,
+            0, 0, 16, 0, 0x8, 0, 0, 0, 1,
             5,       // Pad length
-            "hello", // Data
-            "world", // Padding
+            "helloworld", // Data
+            "\0\0\0\0\0", // Padding
         ];
     };
 
     let data = poll_frame!(Data, codec);
     assert_eq!(data.stream_id(), 1);
-    assert_eq!(data.payload(), &b"hello"[..]);
+    assert_eq!(data.payload(), &b"helloworld"[..]);
     assert!(!data.is_end_stream());
 
     assert_closed!(codec);

tests/h2-tests/tests/flow_control.rs

@@ -1361,3 +1361,60 @@ fn reset_stream_waiting_for_capacity() {
     client.join(srv).wait().unwrap();
 }
 
+#[test]
+fn data_padding() {
+    let _ = ::env_logger::try_init();
+    let (io, srv) = mock::new();
+
+    let mut body = Vec::new();
+    body.push(5);
+    body.extend_from_slice(&[b'z'; 100][..]);
+    body.extend_from_slice(&[b'0'; 5][..]);
+
+    let srv = srv.assert_client_handshake()
+        .unwrap()
+        .recv_settings()
+        .recv_frame(
+            frames::headers(1)
+                .request("GET", "http://example.com/")
+                .eos()
+        )
+        .send_frame(
+            frames::headers(1)
+                .response(200)
+                .field("content-length", 100)
+        )
+        .send_frame(
+            frames::data(1, body)
+                .padded()
+                .eos()
+        )
+        .close();
+
+    let h2 = client::handshake(io)
+        .expect("handshake")
+        .and_then(|(mut client, conn)| {
+            let request = Request::builder()
+                .method(Method::GET)
+                .uri("http://example.com/")
+                .body(())
+                .unwrap();
+
+            // first request is allowed
+            let (response, _) = client.send_request(request, true).unwrap();
+            let fut = response
+                .and_then(|resp| {
+                    assert_eq!(resp.status(), StatusCode::OK);
+                    let body = resp.into_body();
+                    body.concat2()
+                })
+                .map(|bytes| {
+                    assert_eq!(bytes.len(), 100);
+                });
+            conn
+                .expect("client")
+                .join(fut.expect("response"))
+        });
+
+    h2.join(srv).wait().expect("wait");
+}