Remove qstring dependency. (#221)

Instead, rely on Url's built-in query parameter handling. A Request now
accumulates a list of query param pairs, and joins them with a parsed
URL at the time do_call is called.

In the process, remove some getters that rely on parsing the URL.
Adapting these getters was going to be awkward, and they mostly
duplicate things people can readily get by parsing the URL.

Cargo.toml

@@ -28,7 +28,6 @@ base64 = "0.13"
 chunked_transfer = "1.2.0"
 cookie = { version = "0.14", features = ["percent-encode"], optional = true}
 once_cell = "1"
-qstring = "0.7"
 url = "2"
 socks = { version = "0.3.2", optional = true }
 rustls = { version = "0.18", optional = true, features = [] }

src/request.rs

@@ -1,7 +1,6 @@
 use std::fmt;
 use std::io::Read;
 
-use qstring::QString;
 use url::{form_urlencoded, Url};
 
 use crate::agent::Agent;
@@ -33,22 +32,15 @@ pub struct Request {
     url: String,
     return_error_for_status: bool,
     pub(crate) headers: Vec<Header>,
-    pub(crate) query: QString,
+    query_params: Vec<(String, String)>,
 }
 
 impl fmt::Debug for Request {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let (path, query) = self
-            .to_url()
-            .map(|u| {
-                let query = unit::combine_query(&u, &self.query, true);
-                (u.path().to_string(), query)
-            })
-            .unwrap_or_else(|_| ("BAD_URL".to_string(), "BAD_URL".to_string()));
         write!(
             f,
-            "Request({} {}{}, {:?})",
+            "Request({} {} {:?}, {:?})",
-            self.method, path, query, self.headers
+            self.method, self.url, self.query_params, self.headers
         )
     }
 }
@@ -61,7 +53,7 @@ impl Request {
             url,
             headers: vec![],
             return_error_for_status: true,
-            query: QString::default(),
+            query_params: vec![],
         }
     }
 
@@ -86,11 +78,16 @@ impl Request {
         for h in &self.headers {
             h.validate()?;
         }
-        let response = self.to_url().and_then(|url| {
+        let mut url: Url = self
-            let reader = payload.into_read();
+            .url
-            let unit = Unit::new(&self, &url, true, &reader);
+            .parse()
-            unit::connect(&self, unit, true, 0, reader, false)
+            .map_err(|e: url::ParseError| Error::BadUrl(e.to_string()))?;
-        })?;
+        for (name, value) in self.query_params.clone() {
+            url.query_pairs_mut().append_pair(&name, &value);
+        }
+        let reader = payload.into_read();
+        let unit = Unit::new(&self, &url, &reader);
+        let response = unit::connect(&self, unit, true, 0, reader, false)?;
 
         if response.error() && self.return_error_for_status {
             Err(Error::HTTP(response.into()))
@@ -296,22 +293,8 @@ impl Request {
     /// println!("{:?}", r);
     /// ```
     pub fn query(mut self, param: &str, value: &str) -> Self {
-        self.query.add_pair((param, value));
+        self.query_params
-        self
+            .push((param.to_string(), value.to_string()));
-    }
-
-    /// Set query parameters as a string.
-    ///
-    /// For example, to set `?format=json&dest=/login`
-    ///
-    /// ```
-    /// let r = ureq::get("/my_page")
-    ///     .query_str("?format=json&dest=/login")
-    ///     .call();
-    /// println!("{:?}", r);
-    /// ```
-    pub fn query_str(mut self, query: &str) -> Self {
-        self.query.add_str(query);
         self
     }
 
@@ -334,90 +317,6 @@ impl Request {
         self
     }
 
-    /// Get the method this request is using.
-    ///
-    /// Example:
-    /// ```
-    /// let req = ureq::post("/somewhere");
-    /// assert_eq!(req.get_method(), "POST");
-    /// ```
-    pub fn get_method(&self) -> &str {
-        &self.method
-    }
-
-    /// Get the url this request was created with.
-    ///
-    /// This value is not normalized, it is exactly as set.
-    /// It does not contain any added query parameters.
-    ///
-    /// Example:
-    /// ```
-    /// let req = ureq::post("https://cool.server/innit");
-    /// assert_eq!(req.get_url(), "https://cool.server/innit");
-    /// ```
-    pub fn get_url(&self) -> &str {
-        &self.url
-    }
-
-    /// Normalizes and returns the host that will be used for this request.
-    ///
-    /// Example:
-    /// ```
-    /// let req1 = ureq::post("https://cool.server/innit");
-    /// assert_eq!(req1.get_host().unwrap(), "cool.server");
-    ///
-    /// let req2 = ureq::post("http://localhost/some/path");
-    /// assert_eq!(req2.get_host().unwrap(), "localhost");
-    /// ```
-    pub fn get_host(&self) -> Result<String> {
-        match self.to_url() {
-            Ok(u) => match u.host_str() {
-                Some(host) => Ok(host.to_string()),
-                None => Err(Error::BadUrl("No hostname in URL".into())),
-            },
-            Err(e) => Err(e),
-        }
-    }
-
-    /// Returns the scheme for this request.
-    ///
-    /// Example:
-    /// ```
-    /// let req = ureq::post("https://cool.server/innit");
-    /// assert_eq!(req.get_scheme().unwrap(), "https");
-    /// ```
-    pub fn get_scheme(&self) -> Result<String> {
-        self.to_url().map(|u| u.scheme().to_string())
-    }
-
-    /// The complete query for this request.
-    ///
-    /// Example:
-    /// ```
-    /// let req = ureq::post("https://cool.server/innit?foo=bar")
-    ///     .query("format", "json");
-    /// assert_eq!(req.get_query().unwrap(), "?foo=bar&format=json");
-    /// ```
-    pub fn get_query(&self) -> Result<String> {
-        self.to_url()
-            .map(|u| unit::combine_query(&u, &self.query, true))
-    }
-
-    /// The normalized url of this request.
-    ///
-    /// Example:
-    /// ```
-    /// let req = ureq::post("https://cool.server/innit");
-    /// assert_eq!(req.get_path().unwrap(), "/innit");
-    /// ```
-    pub fn get_path(&self) -> Result<String> {
-        self.to_url().map(|u| u.path().to_string())
-    }
-
-    fn to_url(&self) -> Result<Url> {
-        Url::parse(&self.url).map_err(|e| Error::BadUrl(format!("{}", e)))
-    }
-
     // Returns true if this request, with the provided body, is retryable.
     pub(crate) fn is_retryable(&self, body: &SizedReader) -> bool {
         // Per https://tools.ietf.org/html/rfc7231#section-8.1.3
@@ -441,16 +340,6 @@ impl Request {
     }
 }
 
-#[test]
-fn no_hostname() {
-    let req = Request::new(
-        Agent::new(),
-        "GET".to_string(),
-        "unix:/run/foo.socket".to_string(),
-    );
-    assert!(req.get_host().is_err());
-}
-
 #[test]
 fn request_implements_send_and_sync() {
     let _request: Box<dyn Send> = Box::new(Request::new(

src/test/query_string.rs

@@ -25,7 +25,11 @@ fn escaped_query_string() {
         .unwrap();
     let vec = resp.to_write_vec();
     let s = String::from_utf8_lossy(&vec);
-    assert!(s.contains("GET /escaped_query_string?foo=bar&baz=yo%20lo HTTP/1.1"))
+    assert!(
+        s.contains("GET /escaped_query_string?foo=bar&baz=yo+lo HTTP/1.1"),
+        "req: {}",
+        s
+    );
 }
 
 #[test]
@@ -50,5 +54,5 @@ fn query_in_path_and_req() {
         .unwrap();
     let vec = resp.to_write_vec();
     let s = String::from_utf8_lossy(&vec);
-    assert!(s.contains("GET /query_in_path_and_req?foo=bar&baz=1%202%203 HTTP/1.1"))
+    assert!(s.contains("GET /query_in_path_and_req?foo=bar&baz=1+2+3 HTTP/1.1"))
 }

src/test/simple.rs

@@ -136,7 +136,7 @@ fn request_debug() {
 
     assert_eq!(
         s,
-        "Request(GET /my/page, [Authorization: abcdef, \
+        "Request(GET http://localhost/my/page [], [Authorization: abcdef, \
          Content-Length: 1234, Content-Type: application/json])"
     );
 
@@ -148,7 +148,7 @@ fn request_debug() {
 
     assert_eq!(
         s,
-        "Request(GET /my/page?q=z&foo=bar%20baz, [Authorization: abcdef])"
+        "Request(GET http://localhost/my/page?q=z [(\"foo\", \"bar baz\")], [Authorization: abcdef])"
     );
 }
 

src/unit.rs

@@ -2,7 +2,6 @@ use std::io::{self, Write};
 use std::time;
 
 use log::{debug, info};
-use qstring::QString;
 use url::Url;
 
 #[cfg(feature = "cookies")]
@@ -23,7 +22,6 @@ pub(crate) struct Unit {
     pub req: Request,
     pub url: Url,
     pub is_chunked: bool,
-    pub query_string: String,
     pub headers: Vec<Header>,
     pub deadline: Option<time::Instant>,
 }
@@ -31,7 +29,7 @@ pub(crate) struct Unit {
 impl Unit {
     //
 
-    pub(crate) fn new(req: &Request, url: &Url, mix_queries: bool, body: &SizedReader) -> Self {
+    pub(crate) fn new(req: &Request, url: &Url, body: &SizedReader) -> Self {
         //
 
         let (is_transfer_encoding_set, mut is_chunked) = req
@@ -48,8 +46,6 @@ impl Unit {
             // otherwise, no chunking.
             .unwrap_or((false, false));
 
-        let query_string = combine_query(&url, &req.query, mix_queries);
-
         let extra_headers = {
             let mut extra = vec![];
 
@@ -106,7 +102,6 @@ impl Unit {
             req: req.clone(),
             url: url.clone(),
             is_chunked,
-            query_string,
             headers,
             deadline,
         }
@@ -225,7 +220,7 @@ pub(crate) fn connect(
                 301 | 302 | 303 => {
                     let empty = Payload::Empty.into_read();
                     // recreate the unit to get a new hostname and cookies for the new host.
-                    let mut new_unit = Unit::new(req, &new_url, false, &empty);
+                    let mut new_unit = Unit::new(req, &new_url, &empty);
                     // this is to follow how curl does it. POST, PUT etc change
                     // to GET on a redirect.
                     new_unit.req.method = match &method[..] {
@@ -271,16 +266,6 @@ fn extract_cookies(agent: &Agent, url: &Url) -> Option<Header> {
     }
 }
 
-/// Combine the query of the url and the query options set on the request object.
-pub(crate) fn combine_query(url: &Url, query: &QString, mix_queries: bool) -> String {
-    match (url.query(), !query.is_empty() && mix_queries) {
-        (Some(urlq), true) => format!("?{}&{}", urlq, query),
-        (Some(urlq), false) => format!("?{}", urlq),
-        (None, true) => format!("?{}", query),
-        (None, false) => "".to_string(),
-    }
-}
-
 /// Connect the socket, either by using the pool or grab a new one.
 fn connect_socket(unit: &Unit, hostname: &str, use_pooled: bool) -> Result<(Stream, bool), Error> {
     match unit.url.scheme() {
@@ -323,10 +308,11 @@ fn send_prelude(unit: &Unit, stream: &mut Stream, redir: bool) -> io::Result<()>
     // request line
     write!(
         prelude,
-        "{} {}{} HTTP/1.1\r\n",
+        "{} {}{}{} HTTP/1.1\r\n",
         unit.req.method,
         unit.url.path(),
-        &unit.query_string
+        if unit.url.query().is_some() { "?" } else { "" },
+        unit.url.query().unwrap_or_default(),
     )?;
 
     // host header if not set by user.