From ec5b9b9f37dd100428d002dea7b44d8462ca3de8 Mon Sep 17 00:00:00 2001 From: Eugene Diachkin Date: Thu, 17 Aug 2023 17:19:10 +0300 Subject: [PATCH] Make `http -f` display the request headers. Closes #9912 (#10022) # Description As described in https://github.com/nushell/nushell/issues/9912, the `http` command could display the request headers with the `--full` flag, which could help in debugging the requests. This PR adds such functionality. # User-Facing Changes If `http get` or other `http` command which supports the `--full` flag is invoked with the flag, it used to display the `headers` key which contained an table of response headers. Now this key contains two nested keys: `response` and `request`, each of them being a table of the response and request headers accordingly. ![image](https://github.com/nushell/nushell/assets/24980/d3cfc4c3-6c27-4634-8552-2cdfbdfc7076) --- crates/nu-command/src/network/http/client.rs | 132 ++++++++++++------ crates/nu-command/src/network/http/delete.rs | 3 +- crates/nu-command/src/network/http/get.rs | 3 +- crates/nu-command/src/network/http/options.rs | 3 +- crates/nu-command/src/network/http/patch.rs | 3 +- crates/nu-command/src/network/http/post.rs | 3 +- crates/nu-command/src/network/http/put.rs | 3 +- .../tests/commands/network/http/get.rs | 35 +++++ 8 files changed, 137 insertions(+), 48 deletions(-) diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs index 0104b15961..7734d68629 100644 --- a/crates/nu-command/src/network/http/client.rs +++ b/crates/nu-command/src/network/http/client.rs @@ -464,29 +464,46 @@ fn request_handle_response_content( requested_url: &str, flags: RequestFlags, resp: Response, + request: Request, ) -> Result { - let response_headers: Option = if flags.full { - let headers_raw = request_handle_response_headers_raw(span, &resp)?; - Some(headers_raw) - } else { - None + // #response_to_buffer moves "resp" making it impossible to read headers later. + // Wrapping it into a closure to call when needed + let mut consume_response_body = |response: Response| { + let content_type = response.header("content-type").map(|s| s.to_owned()); + + match content_type { + Some(content_type) => transform_response_using_content_type( + engine_state, + stack, + span, + requested_url, + &flags, + response, + &content_type, + ), + None => Ok(response_to_buffer(response, engine_state, span)), + } }; - let response_status = resp.status(); - let content_type = resp.header("content-type").map(|s| s.to_owned()); - let formatted_content = match content_type { - Some(content_type) => transform_response_using_content_type( - engine_state, - stack, - span, - requested_url, - &flags, - resp, - &content_type, - ), - None => Ok(response_to_buffer(resp, engine_state, span)), - }; if flags.full { + let response_status = resp.status(); + + let request_headers_value = match headers_to_nu(&extract_request_headers(&request), span) { + Ok(headers) => headers.into_value(span), + Err(_) => Value::nothing(span), + }; + + let response_headers_value = match headers_to_nu(&extract_response_headers(&resp), span) { + Ok(headers) => headers.into_value(span), + Err(_) => Value::nothing(span), + }; + + let headers = Value::Record { + cols: vec!["request".to_string(), "response".to_string()], + vals: vec![request_headers_value, response_headers_value], + span, + }; + let full_response = Value::Record { cols: vec![ "headers".to_string(), @@ -494,19 +511,16 @@ fn request_handle_response_content( "status".to_string(), ], vals: vec![ - match response_headers { - Some(headers) => headers.into_value(span), - None => Value::nothing(span), - }, - formatted_content?.into_value(span), + headers, + consume_response_body(resp)?.into_value(span), Value::int(response_status as i64, span), ], span, - } - .into_pipeline_data(); - Ok(full_response) + }; + + Ok(full_response.into_pipeline_data()) } else { - Ok(formatted_content?) + Ok(consume_response_body(resp)?) } } @@ -517,11 +531,18 @@ pub fn request_handle_response( requested_url: &str, flags: RequestFlags, response: Result, + request: Request, ) -> Result { match response { - Ok(resp) => { - request_handle_response_content(engine_state, stack, span, requested_url, flags, resp) - } + Ok(resp) => request_handle_response_content( + engine_state, + stack, + span, + requested_url, + flags, + resp, + request, + ), Err(e) => match e { ShellErrorOrRequestError::ShellError(e) => Err(e), ShellErrorOrRequestError::RequestError(_, e) => { @@ -534,6 +555,7 @@ pub fn request_handle_response( requested_url, flags, resp, + request, )?) } else { Err(handle_response_error(span, requested_url, *e)) @@ -546,16 +568,39 @@ pub fn request_handle_response( } } -pub fn request_handle_response_headers_raw( - span: Span, - response: &Response, -) -> Result { - let header_names = response.headers_names(); +type Headers = HashMap>; +fn extract_request_headers(request: &Request) -> Headers { + request + .header_names() + .iter() + .map(|name| { + ( + name.clone(), + request.all(name).iter().map(|e| e.to_string()).collect(), + ) + }) + .collect() +} + +fn extract_response_headers(response: &Response) -> Headers { + response + .headers_names() + .iter() + .map(|name| { + ( + name.clone(), + response.all(name).iter().map(|e| e.to_string()).collect(), + ) + }) + .collect() +} + +fn headers_to_nu(headers: &Headers, span: Span) -> Result { let cols = vec!["name".to_string(), "value".to_string()]; - let mut vals = Vec::with_capacity(header_names.len()); + let mut vals = Vec::with_capacity(headers.len()); - for name in &header_names { + for (name, values) in headers { let is_duplicate = vals.iter().any(|val| { if let Value::Record { vals, .. } = val { if let Some(Value::String { @@ -568,10 +613,13 @@ pub fn request_handle_response_headers_raw( false }); if !is_duplicate { - // Use the ureq `Response.all` api to get all of the header values with a given name. + // A single header can hold multiple values // This interface is why we needed to check if we've already parsed this header name. - for str_value in response.all(name) { - let header = vec![Value::string(name, span), Value::string(str_value, span)]; + for str_value in values { + let header = vec![ + Value::string(name, span), + Value::string(str_value.to_string(), span), + ]; vals.push(Value::record(cols.clone(), header, span)); } } @@ -585,7 +633,7 @@ pub fn request_handle_response_headers( response: Result, ) -> Result { match response { - Ok(resp) => request_handle_response_headers_raw(span, &resp), + Ok(resp) => headers_to_nu(&extract_response_headers(&resp), span), Err(e) => match e { ShellErrorOrRequestError::ShellError(e) => Err(e), ShellErrorOrRequestError::RequestError(requested_url, e) => { diff --git a/crates/nu-command/src/network/http/delete.rs b/crates/nu-command/src/network/http/delete.rs index 5c8bcfe461..83c3991566 100644 --- a/crates/nu-command/src/network/http/delete.rs +++ b/crates/nu-command/src/network/http/delete.rs @@ -194,7 +194,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, args.data, args.content_type, ctrl_c); + let response = send_request(request.clone(), args.data, args.content_type, ctrl_c); let request_flags = RequestFlags { raw: args.raw, @@ -209,6 +209,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/src/network/http/get.rs b/crates/nu-command/src/network/http/get.rs index a8586b9848..eb3ffbed67 100644 --- a/crates/nu-command/src/network/http/get.rs +++ b/crates/nu-command/src/network/http/get.rs @@ -178,7 +178,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, None, None, ctrl_c); + let response = send_request(request.clone(), None, None, ctrl_c); let request_flags = RequestFlags { raw: args.raw, @@ -193,6 +193,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/src/network/http/options.rs b/crates/nu-command/src/network/http/options.rs index 146383cd17..7449fcbbb8 100644 --- a/crates/nu-command/src/network/http/options.rs +++ b/crates/nu-command/src/network/http/options.rs @@ -167,7 +167,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, None, None, ctrl_c); + let response = send_request(request.clone(), None, None, ctrl_c); // http options' response always showed in header, so we set full to true. // And `raw` is useless too because options method doesn't return body, here we set to true @@ -185,6 +185,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/src/network/http/patch.rs b/crates/nu-command/src/network/http/patch.rs index a688b4ef9b..27fc1c024f 100644 --- a/crates/nu-command/src/network/http/patch.rs +++ b/crates/nu-command/src/network/http/patch.rs @@ -184,7 +184,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, Some(args.data), args.content_type, ctrl_c); + let response = send_request(request.clone(), Some(args.data), args.content_type, ctrl_c); let request_flags = RequestFlags { raw: args.raw, @@ -199,6 +199,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/src/network/http/post.rs b/crates/nu-command/src/network/http/post.rs index 78914de426..bcdb0de8a1 100644 --- a/crates/nu-command/src/network/http/post.rs +++ b/crates/nu-command/src/network/http/post.rs @@ -184,7 +184,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, Some(args.data), args.content_type, ctrl_c); + let response = send_request(request.clone(), Some(args.data), args.content_type, ctrl_c); let request_flags = RequestFlags { raw: args.raw, @@ -199,6 +199,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/src/network/http/put.rs b/crates/nu-command/src/network/http/put.rs index 3d79d75eae..be6a26cb50 100644 --- a/crates/nu-command/src/network/http/put.rs +++ b/crates/nu-command/src/network/http/put.rs @@ -184,7 +184,7 @@ fn helper( request = request_add_authorization_header(args.user, args.password, request); request = request_add_custom_headers(args.headers, request)?; - let response = send_request(request, Some(args.data), args.content_type, ctrl_c); + let response = send_request(request.clone(), Some(args.data), args.content_type, ctrl_c); let request_flags = RequestFlags { raw: args.raw, @@ -199,6 +199,7 @@ fn helper( &requested_url, request_flags, response, + request, ) } diff --git a/crates/nu-command/tests/commands/network/http/get.rs b/crates/nu-command/tests/commands/network/http/get.rs index ea4e6fc938..d63f3864d3 100644 --- a/crates/nu-command/tests/commands/network/http/get.rs +++ b/crates/nu-command/tests/commands/network/http/get.rs @@ -138,9 +138,44 @@ fn http_get_with_custom_headers_as_records() { "http get -H {{content-type: text/plain}} {url}", url = server.url() )); + mock1.assert(); mock2.assert(); } + +#[test] +fn http_get_full_response() { + let mut server = Server::new(); + + let _mock = server.mock("GET", "/").with_body("foo").create(); + + let actual = nu!(pipeline( + format!( + "http get --full {url} --headers [foo bar] | to json", + url = server.url() + ) + .as_str() + )); + + let output: serde_json::Value = serde_json::from_str(&actual.out).unwrap(); + + assert_eq!(output["status"], 200); + assert_eq!(output["body"], "foo"); + + // There's only one request header, we can get it by index + assert_eq!(output["headers"]["request"][0]["name"], "foo"); + assert_eq!(output["headers"]["request"][0]["value"], "bar"); + + // ... and multiple response headers, so have to search by name + let header = output["headers"]["response"] + .as_array() + .unwrap() + .iter() + .find(|e| e["name"] == "connection") + .unwrap(); + assert_eq!(header["value"], "close"); +} + // These tests require network access; they use badssl.com which is a Google-affiliated site for testing various SSL errors. // Revisit this if these tests prove to be flaky or unstable.