diff --git a/cli/tests/integration/request.rs b/cli/tests/integration/request.rs index 2e856138..6bdda027 100644 --- a/cli/tests/integration/request.rs +++ b/cli/tests/integration/request.rs @@ -9,3 +9,13 @@ async fn request_works() -> TestResult { assert_eq!(resp.status(), StatusCode::OK); Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn request_works_component() -> TestResult { + let resp = Test::using_fixture("request.wasm") + .adapt_component() + .against_empty() + .await?; + assert_eq!(resp.status(), StatusCode::OK); + Ok(()) +} diff --git a/lib/src/component/headers.rs b/lib/src/component/headers.rs index f2fbbc0e..7e24d20a 100644 --- a/lib/src/component/headers.rs +++ b/lib/src/component/headers.rs @@ -3,18 +3,22 @@ type MultiValueCursor = u32; /// Write multiple values out to a single buffer, until the iterator is exhausted, or `max_len` /// bytes have been written. In the case that there are still values remaining, the second value of /// the returned tuple will be `Some`. +/// +/// If it's not possible to fit a single value inside a buffer of length `max_len`, an error will +/// be returned with the size necessary for the first element of the collection. pub fn write_values( iter: I, terminator: u8, max_len: usize, - mut cursor: MultiValueCursor, -) -> (Vec, Option) + cursor_start: MultiValueCursor, +) -> Result<(Vec, Option), usize> where I: Iterator, T: AsRef<[u8]>, { let mut buf = Vec::with_capacity(max_len); + let mut cursor = cursor_start; let mut finished = true; let skip_amt = usize::try_from(cursor).expect("u32 can fit in usize"); for item in iter.skip(skip_amt) { @@ -22,6 +26,12 @@ where let needed = buf.len() + bytes.len() + 1; if needed > max_len { + // If we haven't written a single entry yet, return an error indicating how much space + // we would need to write a single entry. + if cursor == cursor_start { + return Err(needed); + } + finished = false; break; } @@ -34,5 +44,5 @@ where let cursor = if finished { None } else { Some(cursor) }; - (buf, cursor) + Ok((buf, cursor)) } diff --git a/lib/src/component/http_body.rs b/lib/src/component/http_body.rs index 34b7aabd..e1d43fc4 100644 --- a/lib/src/component/http_body.rs +++ b/lib/src/component/http_body.rs @@ -168,12 +168,17 @@ impl http_body::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); - if buf.is_empty() && next.is_none() { - return Ok(None); - } + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; - Ok(Some((buf, next))) + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. + if buf.is_empty() { + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) + } } async fn trailer_value_get( @@ -241,12 +246,16 @@ impl http_body::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; - if buf.is_empty() && next.is_none() { - return Ok(None); + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. + if buf.is_empty() { + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) } - - Ok(Some((buf, next))) } } diff --git a/lib/src/component/http_req.rs b/lib/src/component/http_req.rs index 9a71556d..0589c5fc 100644 --- a/lib/src/component/http_req.rs +++ b/lib/src/component/http_req.rs @@ -59,11 +59,9 @@ impl http_req::Host for Session { let req_method = &req.method; if req_method.as_str().len() > usize::try_from(max_len).unwrap() { - return Err(Error::BufferLengthError { - buf: "method", - len: "method_max_len", - } - .into()); + return Err(types::Error::BufferLen( + u64::try_from(req_method.as_str().len()).unwrap(), + )); } Ok(req_method.to_string()) @@ -79,11 +77,7 @@ impl http_req::Host for Session { let res = req_uri.to_string(); if res.len() > usize::try_from(max_len).unwrap() { - return Err(Error::BufferLengthError { - buf: "reqid_out", - len: "reqid_max_len", - } - .into()); + return Err(types::Error::BufferLen(u64::try_from(res.len()).unwrap())); } Ok(res) @@ -185,13 +179,17 @@ impl http_req::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; - if buf.is_empty() && next.is_none() { - return Ok(None); + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. + if buf.is_empty() { + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) } - - Ok(Some((buf, next))) } async fn header_value_get( @@ -234,13 +232,17 @@ impl http_req::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; - if buf.is_empty() && next.is_none() { - return Ok(None); + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. + if buf.is_empty() { + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) } - - Ok(Some((buf, next))) } async fn header_values_set( @@ -759,11 +761,9 @@ impl http_req::Host for Session { let result = format!("{:032x}", self.req_id()); if result.len() > usize::try_from(max_len).unwrap() { - return Err(Error::BufferLengthError { - buf: "reqid_out", - len: "reqid_max_len", - } - .into()); + return Err(types::Error::BufferLen( + u64::try_from(result.len()).unwrap(), + )); } Ok(result) diff --git a/lib/src/component/http_resp.rs b/lib/src/component/http_resp.rs index 5618fe01..4992ee7b 100644 --- a/lib/src/component/http_resp.rs +++ b/lib/src/component/http_resp.rs @@ -87,22 +87,17 @@ impl http_resp::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. if buf.is_empty() { - if next.is_none() { - return Ok(None); - } else { - // It's an error if we couldn't write even a single value. - return Err(Error::BufferLengthError { - buf: "buf", - len: "buf.len()", - } - .into()); - } + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) } - - Ok(Some((buf, next))) } async fn header_value_get( @@ -155,22 +150,17 @@ impl http_resp::Host for Session { b'\0', usize::try_from(max_len).unwrap(), cursor, - ); + ) + .map_err(|needed| types::Error::BufferLen(u64::try_from(needed).unwrap_or(0)))?; + // At this point we know that the buffer being empty will also mean that there are no + // remaining entries to read. if buf.is_empty() { - if next.is_none() { - return Ok(None); - } else { - // It's an error if we couldn't write even a single value. - return Err(Error::BufferLengthError { - buf: "buf", - len: "buf.len()", - } - .into()); - } + debug_assert!(next.is_none()); + Ok(None) + } else { + Ok(Some((buf, next))) } - - Ok(Some((buf, next))) } } } diff --git a/lib/src/component/types.rs b/lib/src/component/types.rs index e35ed453..7b290ae1 100644 --- a/lib/src/component/types.rs +++ b/lib/src/component/types.rs @@ -21,6 +21,12 @@ impl types::Host for Session { } } +impl From for TrappableError { + fn from(e: types::Error) -> Self { + Self::Error(e) + } +} + impl From for TrappableError { fn from(_: HandleError) -> Self { Self::Error(types::Error::BadHandle) diff --git a/test-fixtures/src/bin/request.rs b/test-fixtures/src/bin/request.rs index d3d1709e..d28c0fe7 100644 --- a/test-fixtures/src/bin/request.rs +++ b/test-fixtures/src/bin/request.rs @@ -235,11 +235,13 @@ fn test_header_value_get_and_insert() { // Test that an attempt to get a too-long header name fails. nwritten = 0; + let long_header = + Vec::from_iter(hdr_name.iter().cycle().take(HEADER_LEN_TOO_LONG).copied()); assert_eq!( header_value_get( req, - hdr_name.as_ptr(), - HEADER_LEN_TOO_LONG, + long_header.as_ptr(), + long_header.len(), good_buffer.as_mut_ptr(), good_max, &mut nwritten @@ -252,8 +254,8 @@ fn test_header_value_get_and_insert() { assert_eq!( header_insert( req, - hdr_name.as_ptr(), - HEADER_LEN_TOO_LONG, + long_header.as_ptr(), + long_header.len(), hdr_val.as_ptr(), hdr_val.len(), ), @@ -299,11 +301,13 @@ fn test_header_append_and_remove() { ); // Test that an attempt to append a too-long header name fails. + let long_header = + Vec::from_iter(hdr_name.iter().cycle().take(HEADER_LEN_TOO_LONG).copied()); assert_eq!( header_append( req, - hdr_name.as_ptr(), - HEADER_LEN_TOO_LONG, + long_header.as_ptr(), + long_header.len(), hdr_val.as_ptr(), hdr_val.len(), ), @@ -312,7 +316,7 @@ fn test_header_append_and_remove() { // Test that an attempt to remove a too-long header name fails. assert_eq!( - header_remove(req, hdr_name.as_ptr(), HEADER_LEN_TOO_LONG,), + header_remove(req, long_header.as_ptr(), long_header.len()), FastlyStatus::INVAL );