Repository: algesten/ureq
Pull request #471 (2022-01-29 10:17:51)

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.


Patch:
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<u8> {
+        // 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<u8>. This is used for tests only.
+    fn written_bytes(&self) -> Vec<u8> {
+        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<u8> {
+        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<u8> {
+        self.inner.get_ref().written_bytes()
     }
 }
 


Test patch:
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"));
 }