From 4d2d553cca5abd3f6c204c20c1eca0f1df094924 Mon Sep 17 00:00:00 2001
From: Jack Wright <56345+ayax79@users.noreply.github.com>
Date: Thu, 5 Sep 2024 11:08:19 -0700
Subject: [PATCH] Use String::contains instead of exact match when matching
 content types for http requests. (#13791)

# Description
The existing code uses exact matches on content type. This can caused
things like "application/json; charset=utf-8" that contain a charset not
using send_json method.

NOTE: The charset portion in the above example would still be ignored as
we rely on serde and the client library to control the encoding, it is
still better to catch the json case.

---------

Co-authored-by: Ian Manske <ian.manske@pm.me>
---
 crates/nu-command/src/network/http/client.rs | 74 ++++++++++++++++----
 1 file changed, 61 insertions(+), 13 deletions(-)

diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs
index 5c335fedf0..7afd114024 100644
--- a/crates/nu-command/src/network/http/client.rs
+++ b/crates/nu-command/src/network/http/client.rs
@@ -20,12 +20,26 @@ use url::Url;
 
 const HTTP_DOCS: &str = "https://www.nushell.sh/cookbook/http.html";
 
-#[derive(PartialEq, Eq)]
+type ContentType = String;
+
+#[derive(Debug, PartialEq, Eq)]
 pub enum BodyType {
     Json,
     Form,
     Multipart,
-    Unknown,
+    Unknown(Option<ContentType>),
+}
+
+impl From<Option<ContentType>> for BodyType {
+    fn from(content_type: Option<ContentType>) -> Self {
+        match content_type {
+            Some(it) if it.contains("application/json") => BodyType::Json,
+            Some(it) if it.contains("application/x-www-form-urlencoded") => BodyType::Form,
+            Some(it) if it.contains("multipart/form-data") => BodyType::Multipart,
+            Some(it) => BodyType::Unknown(Some(it)),
+            None => BodyType::Unknown(None),
+        }
+    }
 }
 
 #[derive(Clone, Copy, PartialEq)]
@@ -212,15 +226,14 @@ pub fn send_request(
             send_cancellable_request_bytes(&request_url, req, byte_stream, span, signals)
         }
         HttpBody::Value(body) => {
-            let (body_type, req) = match content_type {
-                Some(it) if it == "application/json" => (BodyType::Json, request),
-                Some(it) if it == "application/x-www-form-urlencoded" => (BodyType::Form, request),
-                Some(it) if it == "multipart/form-data" => (BodyType::Multipart, request),
-                Some(it) => {
-                    let r = request.clone().set("Content-Type", &it);
-                    (BodyType::Unknown, r)
-                }
-                _ => (BodyType::Unknown, request),
+            let body_type = BodyType::from(content_type);
+
+            // We should set the content_type if there is one available
+            // when the content type is unknown
+            let req = if let BodyType::Unknown(Some(content_type)) = &body_type {
+                request.clone().set("Content-Type", content_type)
+            } else {
+                request
             };
 
             match body_type {
@@ -229,7 +242,9 @@ pub fn send_request(
                 BodyType::Multipart => {
                     send_multipart_request(&request_url, body, req, span, signals)
                 }
-                BodyType::Unknown => send_default_request(&request_url, body, req, span, signals),
+                BodyType::Unknown(_) => {
+                    send_default_request(&request_url, body, req, span, signals)
+                }
             }
         }
     }
@@ -243,9 +258,14 @@ fn send_json_request(
     signals: &Signals,
 ) -> Result<Response, ShellErrorOrRequestError> {
     let data = match body {
-        Value::Int { .. } | Value::List { .. } | Value::String { .. } | Value::Record { .. } => {
+        Value::Int { .. } | Value::List { .. } | Value::Record { .. } => {
             value_to_json_value(&body)?
         }
+        // If the body type is string, assume it is string json content.
+        // If parsing fails, just send the raw string
+        Value::String { val: s, .. } => {
+            serde_json::from_str(&s).unwrap_or_else(|_| nu_json::Value::String(s))
+        }
         _ => {
             return Err(ShellErrorOrRequestError::ShellError(
                 ShellError::UnsupportedHttpBody {
@@ -861,3 +881,31 @@ fn retrieve_http_proxy_from_env(engine_state: &EngineState, stack: &mut Stack) -
         .or(stack.get_env_var(engine_state, "ALL_PROXY"))
         .and_then(|proxy| proxy.coerce_into_string().ok())
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_body_type_from_content_type() {
+        let json = Some("application/json".to_string());
+        assert_eq!(BodyType::Json, BodyType::from(json));
+
+        // while the charset wont' be passed as we are allowing serde and the library to control
+        // this, it still shouldn't be missed as json if passed in.
+        let json_with_charset = Some("application/json; charset=utf-8".to_string());
+        assert_eq!(BodyType::Json, BodyType::from(json_with_charset));
+
+        let form = Some("application/x-www-form-urlencoded".to_string());
+        assert_eq!(BodyType::Form, BodyType::from(form));
+
+        let multipart = Some("multipart/form-data".to_string());
+        assert_eq!(BodyType::Multipart, BodyType::from(multipart));
+
+        let unknown = Some("application/octet-stream".to_string());
+        assert_eq!(BodyType::Unknown(unknown.clone()), BodyType::from(unknown));
+
+        let none = None;
+        assert_eq!(BodyType::Unknown(none.clone()), BodyType::from(none));
+    }
+}