From 995603b08ccb48205a2dbec24102a9fb0739ddee Mon Sep 17 00:00:00 2001
From: Reilly Wood <26268125+rgwood@users.noreply.github.com>
Date: Wed, 29 Mar 2023 11:55:51 -0700
Subject: [PATCH] Fix record-to-JSON conversion for HTTP commands (#8663)

This PR fixes a bug introduced in
https://github.com/nushell/nushell/pull/8571.

We were accidentally converting a `Result<Value, ShellError>` to JSON
instead of converting a `Value`. The upshot was that we were sending
JSON like `{"Ok":{"foo":"bar"}}` instead of `{"foo":"bar"}`.

This was an easy bug to miss, because `ureq::send_json()` accepts any
`impl serde::Serialize`. I've added a test to prevent regression.
---
 crates/nu-command/src/network/http/client.rs   | 17 +++++++----------
 .../tests/commands/network/http/post.rs        | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs
index 94162f51b..263ddc09e 100644
--- a/crates/nu-command/src/network/http/client.rs
+++ b/crates/nu-command/src/network/http/client.rs
@@ -141,8 +141,10 @@ pub enum ShellErrorOrRequestError {
     RequestError(String, Error),
 }
 
-fn wrap_shell_error(err: ShellError) -> ShellErrorOrRequestError {
-    ShellErrorOrRequestError::ShellError(err)
+impl From<ShellError> for ShellErrorOrRequestError {
+    fn from(error: ShellError) -> Self {
+        ShellErrorOrRequestError::ShellError(error)
+    }
 }
 
 pub fn send_request(
@@ -174,14 +176,14 @@ pub fn send_request(
             ctrl_c,
         ),
         Value::Record { .. } if body_type == BodyType::Json => {
-            let data = value_to_json_value(&body);
+            let data = value_to_json_value(&body)?;
             send_cancellable_request(&request_url, Box::new(|| request.send_json(data)), ctrl_c)
         }
         Value::Record { cols, vals, .. } if body_type == BodyType::Form => {
             let mut data: Vec<(String, String)> = Vec::with_capacity(cols.len());
 
             for (col, val) in cols.iter().zip(vals.iter()) {
-                let val_string = val.as_string().map_err(wrap_shell_error)?;
+                let val_string = val.as_string()?;
                 data.push((col.clone(), val_string))
             }
 
@@ -204,12 +206,7 @@ pub fn send_request(
 
             let data = vals
                 .chunks(2)
-                .map(|it| {
-                    Ok((
-                        it[0].as_string().map_err(wrap_shell_error)?,
-                        it[1].as_string().map_err(wrap_shell_error)?,
-                    ))
-                })
+                .map(|it| Ok((it[0].as_string()?, it[1].as_string()?)))
                 .collect::<Result<Vec<(String, String)>, ShellErrorOrRequestError>>()?;
 
             let request_fn = move || {
diff --git a/crates/nu-command/tests/commands/network/http/post.rs b/crates/nu-command/tests/commands/network/http/post.rs
index 5f56121f5..820ee19aa 100644
--- a/crates/nu-command/tests/commands/network/http/post.rs
+++ b/crates/nu-command/tests/commands/network/http/post.rs
@@ -76,3 +76,21 @@ fn http_post_failed_due_to_unexpected_body() {
 
     assert!(actual.err.contains("Cannot make request"))
 }
+
+#[test]
+fn http_post_json_is_success() {
+    let mut server = Server::new();
+
+    let mock = server
+        .mock("POST", "/")
+        .match_body(r#"{"foo":"bar"}"#)
+        .create();
+
+    let actual = nu!(format!(
+        r#"http post -t 'application/json' {url} {{foo: 'bar'}}"#,
+        url = server.url()
+    ));
+
+    mock.assert();
+    assert!(actual.out.is_empty())
+}