From 219b5edf9efd9ff98a9a374ef74a5e80048656f8 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sat, 29 Jan 2022 11:06:29 +0100 Subject: [PATCH] Rename test function as_write_vec -> into_written_bytes Tests use `Response::as_write_vec` to inspect the outgoing HTTP/1.1 request line and headers. The current version has two problems: 1. Called `as_write_vec` when it actually returns a `&[u8]`. 2. Inspects/uses the `Response::stream` without consuming `Response`. The first problem is trivial, but the second is subtle. Currently all calls on `Response` that works with the internal `Response::stream` consumes `self` (`into_string`, `into_reader`). `Response` is by itself `Send + Sync`, and must be so because the nested Stream is `Read + Write + Send + Sync`. However for implementors of `TLSStream`, it would be nice to relax the `Sync` requirement. Assumption: If all fields in Response are `Sync` except `Response::stream`, but any access to `stream` consumes `Response`, we can consider the entire `Response` `Sync`. This assumption can help us relax the `TlsStream` `Sync` requirement in a later PR. --- src/response.rs | 5 +++-- src/stream.rs | 16 ++++++++++------ src/test/body_send.rs | 14 +++++++------- src/test/query_string.rs | 8 ++++---- src/test/simple.rs | 6 +++--- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/response.rs b/src/response.rs index de209d0..128b5c8 100644 --- a/src/response.rs +++ b/src/response.rs @@ -520,8 +520,9 @@ impl Response { } #[cfg(test)] - pub fn as_write_vec(&self) -> &[u8] { - self.stream.as_write_vec() + pub fn into_written_bytes(self) -> Vec { + // Deliberately consume `self` so that any access to `self.stream` must be non-shared. + self.stream.written_bytes() } #[cfg(test)] diff --git a/src/stream.rs b/src/stream.rs index 1b64f0f..10e25d6 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -37,8 +37,10 @@ pub(crate) struct Stream { trait Inner: Read + Write { fn is_poolable(&self) -> bool; fn socket(&self) -> Option<&TcpStream>; - fn as_write_vec(&self) -> &[u8] { - panic!("as_write_vec on non Test stream"); + + /// The bytes written to the stream as a Vec. This is used for tests only. + fn written_bytes(&self) -> Vec { + panic!("written_bytes on non Test stream"); } } @@ -76,8 +78,10 @@ impl Inner for TestStream { fn socket(&self) -> Option<&TcpStream> { None } - fn as_write_vec(&self) -> &[u8] { - &self.1 + + /// For tests only + fn written_bytes(&self) -> Vec { + self.1.clone() } } @@ -274,8 +278,8 @@ impl Stream { } #[cfg(test)] - pub fn as_write_vec(&self) -> &[u8] { - self.inner.get_ref().as_write_vec() + pub fn written_bytes(&self) -> Vec { + self.inner.get_ref().written_bytes() } } diff --git a/src/test/body_send.rs b/src/test/body_send.rs index 12f23e4..76ae004 100644 --- a/src/test/body_send.rs +++ b/src/test/body_send.rs @@ -10,7 +10,7 @@ fn content_length_on_str() { let resp = post("test://host/content_length_on_str") .send_string("Hello World!!!") .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nContent-Length: 14\r\n")); } @@ -24,7 +24,7 @@ fn user_set_content_length_on_str() { .set("Content-Length", "12345") .send_string("Hello World!!!") .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nContent-Length: 12345\r\n")); } @@ -43,7 +43,7 @@ fn content_length_on_json() { let resp = post("test://host/content_length_on_json") .send_json(serde_json::Value::Object(json)) .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nContent-Length: 20\r\n")); } @@ -57,7 +57,7 @@ fn content_length_and_chunked() { .set("Transfer-Encoding", "chunked") .send_string("Hello World!!!") .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("Transfer-Encoding: chunked\r\n")); assert!(!s.contains("\r\nContent-Length:\r\n")); @@ -73,7 +73,7 @@ fn str_with_encoding() { .set("Content-Type", "text/plain; charset=iso-8859-1") .send_string("Hällo Wörld!!!") .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); assert_eq!( &vec[vec.len() - 14..], //H ä l l o _ W ö r l d ! ! ! @@ -95,7 +95,7 @@ fn content_type_on_json() { let resp = post("test://host/content_type_on_json") .send_json(serde_json::Value::Object(json)) .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nContent-Type: application/json\r\n")); } @@ -115,7 +115,7 @@ fn content_type_not_overriden_on_json() { .set("content-type", "text/plain") .send_json(serde_json::Value::Object(json)) .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\ncontent-type: text/plain\r\n")); } diff --git a/src/test/query_string.rs b/src/test/query_string.rs index f1fba8c..4ab8f21 100644 --- a/src/test/query_string.rs +++ b/src/test/query_string.rs @@ -8,7 +8,7 @@ fn no_query_string() { test::make_response(200, "OK", vec![], vec![]) }); let resp = get("test://host/no_query_string").call().unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("GET /no_query_string HTTP/1.1")) } @@ -23,7 +23,7 @@ fn escaped_query_string() { .query("baz", "yo lo") .call() .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!( s.contains("GET /escaped_query_string?foo=bar&baz=yo+lo HTTP/1.1"), @@ -38,7 +38,7 @@ fn query_in_path() { test::make_response(200, "OK", vec![], vec![]) }); let resp = get("test://host/query_in_path?foo=bar").call().unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("GET /query_in_path?foo=bar HTTP/1.1")) } @@ -52,7 +52,7 @@ fn query_in_path_and_req() { .query("baz", "1 2 3") .call() .unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("GET /query_in_path_and_req?foo=bar&baz=1+2+3 HTTP/1.1")) } diff --git a/src/test/simple.rs b/src/test/simple.rs index 29829cf..3391c20 100644 --- a/src/test/simple.rs +++ b/src/test/simple.rs @@ -120,7 +120,7 @@ fn escape_path() { test::make_response(200, "OK", vec![], vec![]) }); let resp = get("test://host/escape_path here").call().unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("GET /escape_path%20here HTTP/1.1")) } @@ -198,7 +198,7 @@ pub fn host_no_port() { test::make_response(200, "OK", vec![], vec![]) }); let resp = get("test://myhost/host_no_port").call().unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nHost: myhost\r\n")); } @@ -209,7 +209,7 @@ pub fn host_with_port() { test::make_response(200, "OK", vec![], vec![]) }); let resp = get("test://myhost:234/host_with_port").call().unwrap(); - let vec = resp.as_write_vec(); + let vec = resp.into_written_bytes(); let s = String::from_utf8_lossy(&vec); assert!(s.contains("\r\nHost: myhost:234\r\n")); }