document all remaining unsafe usages (#1642)

adds some debug assertions where appropriate

actix-http/src/h1/decoder.rs

@@ -76,12 +76,14 @@ pub(crate) trait MessageType: Sized {
                 let name =
                     HeaderName::from_bytes(&slice[idx.name.0..idx.name.1]).unwrap();
 
-                // SAFETY: httparse checks header value is valid UTF-8
+                // SAFETY: httparse already checks header value is only visible ASCII bytes
+                // from_maybe_shared_unchecked contains debug assertions so they are omitted here
                 let value = unsafe {
                     HeaderValue::from_maybe_shared_unchecked(
                         slice.slice(idx.value.0..idx.value.1),
                     )
                 };
+
                 match name {
                     header::CONTENT_LENGTH => {
                         if let Ok(s) = value.to_str() {

actix-http/src/h1/dispatcher.rs

@@ -314,11 +314,15 @@ where
                 Poll::Ready(Err(err)) => return Err(DispatchError::Io(err)),
             }
         }
+
         if written == write_buf.len() {
+            // SAFETY: setting length to 0 is safe
+            // skips one length check vs truncate
             unsafe { write_buf.set_len(0) }
         } else {
             write_buf.advance(written);
         }
+
         Ok(false)
     }
 

actix-http/src/h1/encoder.rs

@@ -129,89 +129,133 @@ pub(crate) trait MessageType: Sized {
             .chain(extra_headers.inner.iter());
 
         // write headers
-        let mut pos = 0;
+
         let mut has_date = false;
-        let mut remaining = dst.capacity() - dst.len();
+
         let mut buf = dst.bytes_mut().as_mut_ptr() as *mut u8;
+        let mut remaining = dst.capacity() - dst.len();
+
+        // tracks bytes written since last buffer resize
+        // since buf is a raw pointer to a bytes container storage but is written to without the
+        // container's knowledge, this is used to sync the containers cursor after data is written
+        let mut pos = 0;
+
         for (key, value) in headers {
             match *key {
                 CONNECTION => continue,
                 TRANSFER_ENCODING | CONTENT_LENGTH if skip_len => continue,
-                DATE => {
-                    has_date = true;
-                }
+                DATE => has_date = true,
                 _ => (),
             }
+
             let k = key.as_str().as_bytes();
+            let k_len = k.len();
+
             match value {
                 map::Value::One(ref val) => {
                     let v = val.as_ref();
                     let v_len = v.len();
-                    let k_len = k.len();
+
+                    // key length + value length + colon + space + \r\n
                     let len = k_len + v_len + 4;
+
                     if len > remaining {
+                        // not enough room in buffer for this header; reserve more space
+
+                        // SAFETY: all the bytes written up to position "pos" are initialized
+                        // the written byte count and pointer advancement are kept in sync
                         unsafe {
                             dst.advance_mut(pos);
                         }
+
                         pos = 0;
                         dst.reserve(len * 2);
                         remaining = dst.capacity() - dst.len();
+
+                        // re-assign buf raw pointer since it's possible that the buffer was
+                        // reallocated and/or resized
                         buf = dst.bytes_mut().as_mut_ptr() as *mut u8;
                     }
-                    // use upper Camel-Case
+
+                    // SAFETY: on each write, it is enough to ensure that the advancement of the
+                    // cursor matches the number of bytes written
                     unsafe {
+                        // use upper Camel-Case
                         if camel_case {
                             write_camel_case(k, from_raw_parts_mut(buf, k_len))
                         } else {
                             write_data(k, buf, k_len)
                         }
+
                         buf = buf.add(k_len);
+
                         write_data(b": ", buf, 2);
                         buf = buf.add(2);
+
                         write_data(v, buf, v_len);
                         buf = buf.add(v_len);
+
                         write_data(b"\r\n", buf, 2);
                         buf = buf.add(2);
-                        pos += len;
-                        remaining -= len;
                     }
+
+                    pos += len;
+                    remaining -= len;
                 }
+
                 map::Value::Multi(ref vec) => {
                     for val in vec {
                         let v = val.as_ref();
                         let v_len = v.len();
-                        let k_len = k.len();
                         let len = k_len + v_len + 4;
+
                         if len > remaining {
+                            // SAFETY: all the bytes written up to position "pos" are initialized
+                            // the written byte count and pointer advancement are kept in sync
                             unsafe {
                                 dst.advance_mut(pos);
                             }
                             pos = 0;
                             dst.reserve(len * 2);
                             remaining = dst.capacity() - dst.len();
+
+                            // re-assign buf raw pointer since it's possible that the buffer was
+                            // reallocated and/or resized
                             buf = dst.bytes_mut().as_mut_ptr() as *mut u8;
                         }
-                        // use upper Camel-Case
+
+                        // SAFETY: on each write, it is enough to ensure that the advancement of
+                        // the cursor matches the number of bytes written
                         unsafe {
                             if camel_case {
                                 write_camel_case(k, from_raw_parts_mut(buf, k_len));
                             } else {
                                 write_data(k, buf, k_len);
                             }
+
                             buf = buf.add(k_len);
+
                             write_data(b": ", buf, 2);
                             buf = buf.add(2);
+
                             write_data(v, buf, v_len);
                             buf = buf.add(v_len);
+
                             write_data(b"\r\n", buf, 2);
                             buf = buf.add(2);
                         };
+
                         pos += len;
                         remaining -= len;
                     }
                 }
             }
         }
+
+        // final cursor synchronization with the bytes container
+        //
+        // SAFETY: all the bytes written up to position "pos" are initialized
+        // the written byte count and pointer advancement are kept in sync
         unsafe {
             dst.advance_mut(pos);
         }
@@ -477,7 +521,10 @@ impl<'a> io::Write for Writer<'a> {
     }
 }
 
+/// # Safety
+/// Callers must ensure that the given length matches given value length.
 unsafe fn write_data(value: &[u8], buf: *mut u8, len: usize) {
+    debug_assert_eq!(value.len(), len);
     copy_nonoverlapping(value.as_ptr(), buf, len);
 }