Factorize HTTP commands code (#8088)

# Description

In order to work on https://github.com/nushell/nushell/issues/2741, I'm
preparing the code.

# User-Facing Changes

Both commands do support the timeout option.

**But the timeout argument `-t, --timeout` has been migrated to `-m,
--max-time`**. I had to make a choice since there is another option
using the short command `t` which is "content-type".

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

crates/nu-command/src/network/http/client.rs

@@ -1,3 +1,25 @@
+use crate::formats::value_to_json_value;
+use base64::engine::general_purpose::PAD;
+use base64::engine::GeneralPurpose;
+use base64::{alphabet, Engine};
+use nu_protocol::ast::Call;
+use nu_protocol::engine::{EngineState, Stack};
+use nu_protocol::{BufferedReader, PipelineData, RawStream, ShellError, Span, Value};
+use reqwest::blocking::{RequestBuilder, Response};
+use reqwest::{blocking, Error, StatusCode};
+use std::collections::HashMap;
+use std::io::BufReader;
+use std::path::PathBuf;
+use std::str::FromStr;
+use std::time::Duration;
+
+#[derive(PartialEq, Eq)]
+pub enum BodyType {
+    Json,
+    Form,
+    Unknown,
+}
+
 // Only panics if the user agent is invalid but we define it statically so either
 // it always or never fails
 pub fn http_client(allow_insecure: bool) -> reqwest::blocking::Client {
@@ -7,3 +29,332 @@ pub fn http_client(allow_insecure: bool) -> reqwest::blocking::Client {
         .build()
         .expect("Failed to build reqwest client")
 }
+
+pub fn response_to_buffer(
+    response: blocking::Response,
+    engine_state: &EngineState,
+    span: Span,
+) -> PipelineData {
+    // Try to get the size of the file to be downloaded.
+    // This is helpful to show the progress of the stream.
+    let buffer_size = match &response.headers().get("content-length") {
+        Some(content_length) => {
+            let content_length = &(*content_length).clone(); // binding
+
+            let content_length = content_length
+                .to_str()
+                .unwrap_or("")
+                .parse::<u64>()
+                .unwrap_or(0);
+
+            if content_length == 0 {
+                None
+            } else {
+                Some(content_length)
+            }
+        }
+        _ => None,
+    };
+
+    let buffered_input = BufReader::new(response);
+
+    PipelineData::ExternalStream {
+        stdout: Some(RawStream::new(
+            Box::new(BufferedReader {
+                input: buffered_input,
+            }),
+            engine_state.ctrlc.clone(),
+            span,
+            buffer_size,
+        )),
+        stderr: None,
+        exit_code: None,
+        span,
+        metadata: None,
+        trim_end_newline: false,
+    }
+}
+
+pub fn request_add_authorization_header(
+    user: Option<String>,
+    password: Option<String>,
+    mut request: RequestBuilder,
+) -> RequestBuilder {
+    let base64_engine = GeneralPurpose::new(&alphabet::STANDARD, PAD);
+
+    let login = match (user, password) {
+        (Some(user), Some(password)) => {
+            let mut enc_str = String::new();
+            base64_engine.encode_string(&format!("{user}:{password}"), &mut enc_str);
+            Some(enc_str)
+        }
+        (Some(user), _) => {
+            let mut enc_str = String::new();
+            base64_engine.encode_string(&format!("{user}:"), &mut enc_str);
+            Some(enc_str)
+        }
+        (_, Some(password)) => {
+            let mut enc_str = String::new();
+            base64_engine.encode_string(&format!(":{password}"), &mut enc_str);
+            Some(enc_str)
+        }
+        _ => None,
+    };
+
+    if let Some(login) = login {
+        request = request.header("Authorization", format!("Basic {login}"));
+    }
+
+    request
+}
+
+pub fn request_set_body(
+    content_type: Option<String>,
+    content_length: Option<String>,
+    body: Value,
+    mut request: RequestBuilder,
+) -> Result<RequestBuilder, ShellError> {
+    // set the content-type header before using e.g., request.json
+    // because that will avoid duplicating the header value
+    if let Some(val) = &content_type {
+        request = request.header("Content-Type", val);
+    }
+
+    let body_type = match content_type {
+        Some(it) if it == "application/json" => BodyType::Json,
+        Some(it) if it == "application/x-www-form-urlencoded" => BodyType::Form,
+        _ => BodyType::Unknown,
+    };
+
+    match body {
+        Value::Binary { val, .. } => {
+            request = request.body(val);
+        }
+        Value::String { val, .. } => {
+            request = request.body(val);
+        }
+        Value::Record { .. } if body_type == BodyType::Json => {
+            let data = value_to_json_value(&body)?;
+            request = request.json(&data);
+        }
+        Value::Record { .. } if body_type == BodyType::Form => {
+            let data = value_to_json_value(&body)?;
+            request = request.form(&data);
+        }
+        Value::List { vals, .. } if body_type == BodyType::Form => {
+            if vals.len() % 2 != 0 {
+                return Err(ShellError::IOError("unsupported body input".into()));
+            }
+            let data = vals
+                .chunks(2)
+                .map(|it| Ok((it[0].as_string()?, it[1].as_string()?)))
+                .collect::<Result<Vec<(String, String)>, ShellError>>()?;
+            request = request.form(&data)
+        }
+        _ => {
+            return Err(ShellError::IOError("unsupported body input".into()));
+        }
+    };
+
+    if let Some(val) = content_length {
+        request = request.header("Content-Length", val);
+    }
+
+    Ok(request)
+}
+
+pub fn request_set_timeout(
+    timeout: Option<Value>,
+    mut request: RequestBuilder,
+) -> Result<RequestBuilder, ShellError> {
+    if let Some(timeout) = timeout {
+        let val = timeout.as_i64()?;
+        if val.is_negative() || val < 1 {
+            return Err(ShellError::TypeMismatch(
+                "Timeout value must be an integer and larger than 0".to_string(),
+                // timeout is already guaranteed to not be an error
+                timeout.expect_span(),
+            ));
+        }
+
+        request = request.timeout(Duration::from_secs(val as u64));
+    }
+
+    Ok(request)
+}
+
+pub fn request_add_custom_headers(
+    headers: Option<Value>,
+    mut request: RequestBuilder,
+) -> Result<RequestBuilder, ShellError> {
+    if let Some(headers) = headers {
+        let mut custom_headers: HashMap<String, Value> = HashMap::new();
+
+        match &headers {
+            Value::List { vals: table, .. } => {
+                if table.len() == 1 {
+                    // single row([key1 key2]; [val1 val2])
+                    match &table[0] {
+                        Value::Record { cols, vals, .. } => {
+                            for (k, v) in cols.iter().zip(vals.iter()) {
+                                custom_headers.insert(k.to_string(), v.clone());
+                            }
+                        }
+
+                        x => {
+                            return Err(ShellError::CantConvert(
+                                "string list or single row".into(),
+                                x.get_type().to_string(),
+                                headers.span().unwrap_or_else(|_| Span::new(0, 0)),
+                                None,
+                            ));
+                        }
+                    }
+                } else {
+                    // primitive values ([key1 val1 key2 val2])
+                    for row in table.chunks(2) {
+                        if row.len() == 2 {
+                            custom_headers.insert(row[0].as_string()?, row[1].clone());
+                        }
+                    }
+                }
+            }
+
+            x => {
+                return Err(ShellError::CantConvert(
+                    "string list or single row".into(),
+                    x.get_type().to_string(),
+                    headers.span().unwrap_or_else(|_| Span::new(0, 0)),
+                    None,
+                ));
+            }
+        };
+
+        for (k, v) in &custom_headers {
+            if let Ok(s) = v.as_string() {
+                request = request.header(k, s);
+            }
+        }
+    }
+
+    Ok(request)
+}
+
+pub fn request_handle_response(
+    engine_state: &EngineState,
+    stack: &mut Stack,
+    span: Span,
+    requested_url: &String,
+    raw: bool,
+    response: Result<Response, Error>,
+) -> Result<PipelineData, ShellError> {
+    // Explicitly turn 4xx and 5xx statuses into errors.
+    match response {
+        Ok(resp) => match resp.headers().get("content-type") {
+            Some(content_type) => {
+                let content_type = content_type.to_str().map_err(|e| {
+                    ShellError::GenericError(
+                        e.to_string(),
+                        "".to_string(),
+                        None,
+                        Some("MIME type were invalid".to_string()),
+                        Vec::new(),
+                    )
+                })?;
+                let content_type = mime::Mime::from_str(content_type).map_err(|_| {
+                    ShellError::GenericError(
+                        format!("MIME type unknown: {content_type}"),
+                        "".to_string(),
+                        None,
+                        Some("given unknown MIME type".to_string()),
+                        Vec::new(),
+                    )
+                })?;
+                let ext = match (content_type.type_(), content_type.subtype()) {
+                    (mime::TEXT, mime::PLAIN) => {
+                        let path_extension = url::Url::parse(requested_url)
+                            .map_err(|_| {
+                                ShellError::GenericError(
+                                    format!("Cannot parse URL: {requested_url}"),
+                                    "".to_string(),
+                                    None,
+                                    Some("cannot parse".to_string()),
+                                    Vec::new(),
+                                )
+                            })?
+                            .path_segments()
+                            .and_then(|segments| segments.last())
+                            .and_then(|name| if name.is_empty() { None } else { Some(name) })
+                            .and_then(|name| {
+                                PathBuf::from(name)
+                                    .extension()
+                                    .map(|name| name.to_string_lossy().to_string())
+                            });
+                        path_extension
+                    }
+                    _ => Some(content_type.subtype().to_string()),
+                };
+
+                let output = response_to_buffer(resp, engine_state, span);
+
+                if raw {
+                    return Ok(output);
+                }
+
+                if let Some(ext) = ext {
+                    match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
+                        Some(converter_id) => engine_state.get_decl(converter_id).run(
+                            engine_state,
+                            stack,
+                            &Call::new(span),
+                            output,
+                        ),
+                        None => Ok(output),
+                    }
+                } else {
+                    Ok(output)
+                }
+            }
+            None => Ok(response_to_buffer(resp, engine_state, span)),
+        },
+        Err(e) if e.is_timeout() => Err(ShellError::NetworkFailure(
+            format!("Request to {requested_url} has timed out"),
+            span,
+        )),
+        Err(e) if e.is_status() => match e.status() {
+            Some(err_code) if err_code == StatusCode::NOT_FOUND => Err(ShellError::NetworkFailure(
+                format!("Requested file not found (404): {requested_url:?}"),
+                span,
+            )),
+            Some(err_code) if err_code == StatusCode::MOVED_PERMANENTLY => {
+                Err(ShellError::NetworkFailure(
+                    format!("Resource moved permanently (301): {requested_url:?}"),
+                    span,
+                ))
+            }
+            Some(err_code) if err_code == StatusCode::BAD_REQUEST => Err(
+                ShellError::NetworkFailure(format!("Bad request (400) to {requested_url:?}"), span),
+            ),
+            Some(err_code) if err_code == StatusCode::FORBIDDEN => Err(ShellError::NetworkFailure(
+                format!("Access forbidden (403) to {requested_url:?}"),
+                span,
+            )),
+            _ => Err(ShellError::NetworkFailure(
+                format!(
+                    "Cannot make request to {:?}. Error is {:?}",
+                    requested_url,
+                    e.to_string()
+                ),
+                span,
+            )),
+        },
+        Err(e) => Err(ShellError::NetworkFailure(
+            format!(
+                "Cannot make request to {:?}. Error is {:?}",
+                requested_url,
+                e.to_string()
+            ),
+            span,
+        )),
+    }
+}

crates/nu-command/src/network/http/get.rs

@@ -1,20 +1,13 @@
-use crate::network::http::client::http_client;
-use base64::{alphabet, engine::general_purpose::PAD, engine::GeneralPurpose, Engine};
+use crate::network::http::client::{
+    http_client, request_add_authorization_header, request_add_custom_headers,
+    request_handle_response, request_set_timeout,
+};
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::util::BufferedReader;
-use nu_protocol::RawStream;
 use nu_protocol::{
-    Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value,
+    Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type, Value,
 };
-use reqwest::blocking::Response;
-use reqwest::StatusCode;
-use std::collections::HashMap;
-use std::io::BufReader;
-use std::path::PathBuf;
-use std::str::FromStr;
-use std::time::Duration;
 
 #[derive(Clone)]
 pub struct SubCommand;
@@ -45,10 +38,10 @@ impl Command for SubCommand {
                 Some('p'),
             )
             .named(
-                "timeout",
+                "max-time",
                 SyntaxShape::Int,
                 "timeout period in seconds",
-                Some('t'),
+                Some('m'),
             )
             .named(
                 "headers",
@@ -91,7 +84,7 @@ impl Command for SubCommand {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        run_fetch(engine_state, stack, call, input)
+        run_post(engine_state, stack, call, input)
     }
 
     fn examples(&self) -> Vec<Example> {
@@ -125,7 +118,7 @@ struct Arguments {
     headers: Option<Value>,
 }
 
-fn run_fetch(
+fn run_post(
     engine_state: &EngineState,
     stack: &mut Stack,
     call: &Call,
@@ -149,9 +142,13 @@ fn helper(
     engine_state: &EngineState,
     stack: &mut Stack,
     args: Arguments,
-) -> std::result::Result<PipelineData, ShellError> {
-    // There is no need to error-check this, as the URL is already guaranteed by basic nu command argument type checks.
+) -> Result<PipelineData, ShellError> {
     let url_value = args.url;
+    let user = args.user.clone();
+    let password = args.password;
+    let timeout = args.timeout;
+    let headers = args.headers;
+    let raw = args.raw;
 
     let span = url_value.span()?;
     let requested_url = url_value.as_string()?;
@@ -165,253 +162,14 @@ fn helper(
             ));
         }
     };
-    let user = args.user.clone();
-    let password = args.password;
-    let timeout = args.timeout;
-    let headers = args.headers;
-    let raw = args.raw;
-    let base64_engine = GeneralPurpose::new(&alphabet::STANDARD, PAD);
-
-    let login = match (user, password) {
-        (Some(user), Some(password)) => {
-            let mut enc_str = String::new();
-            base64_engine.encode_string(&format!("{user}:{password}"), &mut enc_str);
-            Some(enc_str)
-        }
-        (Some(user), _) => {
-            let mut enc_str = String::new();
-            base64_engine.encode_string(&format!("{user}:"), &mut enc_str);
-            Some(enc_str)
-        }
-        (_, Some(password)) => {
-            let mut enc_str = String::new();
-            base64_engine.encode_string(&format!(":{password}"), &mut enc_str);
-            Some(enc_str)
-        }
-        _ => None,
-    };
 
     let client = http_client(args.insecure.is_some());
     let mut request = client.get(url);
 
-    if let Some(timeout) = timeout {
-        let val = timeout.as_i64()?;
-        if val.is_negative() || val < 1 {
-            return Err(ShellError::TypeMismatch(
-                "Timeout value must be an integer and larger than 0".to_string(),
-                // timeout is already guaranteed to not be an error
-                timeout.expect_span(),
-            ));
-        }
+    request = request_set_timeout(timeout, request)?;
+    request = request_add_authorization_header(user, password, request);
+    request = request_add_custom_headers(headers, request)?;
 
-        request = request.timeout(Duration::from_secs(val as u64));
-    }
-
-    if let Some(login) = login {
-        request = request.header("Authorization", format!("Basic {login}"));
-    }
-
-    if let Some(headers) = headers {
-        let mut custom_headers: HashMap<String, Value> = HashMap::new();
-
-        match &headers {
-            Value::List { vals: table, .. } => {
-                if table.len() == 1 {
-                    // single row([key1 key2]; [val1 val2])
-                    match &table[0] {
-                        Value::Record { cols, vals, .. } => {
-                            for (k, v) in cols.iter().zip(vals.iter()) {
-                                custom_headers.insert(k.to_string(), v.clone());
-                            }
-                        }
-
-                        x => {
-                            return Err(ShellError::CantConvert(
-                                "string list or single row".into(),
-                                x.get_type().to_string(),
-                                headers.span().unwrap_or_else(|_| Span::new(0, 0)),
-                                None,
-                            ));
-                        }
-                    }
-                } else {
-                    // primitive values ([key1 val1 key2 val2])
-                    for row in table.chunks(2) {
-                        if row.len() == 2 {
-                            custom_headers.insert(row[0].as_string()?, row[1].clone());
-                        }
-                    }
-                }
-            }
-
-            x => {
-                return Err(ShellError::CantConvert(
-                    "string list or single row".into(),
-                    x.get_type().to_string(),
-                    headers.span().unwrap_or_else(|_| Span::new(0, 0)),
-                    None,
-                ));
-            }
-        };
-
-        for (k, v) in &custom_headers {
-            if let Ok(s) = v.as_string() {
-                request = request.header(k, s);
-            }
-        }
-    }
-
-    // Explicitly turn 4xx and 5xx statuses into errors.
-    match request.send().and_then(|r| r.error_for_status()) {
-        Ok(resp) => match resp.headers().get("content-type") {
-            Some(content_type) => {
-                let content_type = content_type.to_str().map_err(|e| {
-                    ShellError::GenericError(
-                        e.to_string(),
-                        "".to_string(),
-                        None,
-                        Some("MIME type were invalid".to_string()),
-                        Vec::new(),
-                    )
-                })?;
-                let content_type = mime::Mime::from_str(content_type).map_err(|_| {
-                    ShellError::GenericError(
-                        format!("MIME type unknown: {content_type}"),
-                        "".to_string(),
-                        None,
-                        Some("given unknown MIME type".to_string()),
-                        Vec::new(),
-                    )
-                })?;
-                let ext = match (content_type.type_(), content_type.subtype()) {
-                    (mime::TEXT, mime::PLAIN) => {
-                        let path_extension = url::Url::parse(&requested_url)
-                            .map_err(|_| {
-                                ShellError::GenericError(
-                                    format!("Cannot parse URL: {requested_url}"),
-                                    "".to_string(),
-                                    None,
-                                    Some("cannot parse".to_string()),
-                                    Vec::new(),
-                                )
-                            })?
-                            .path_segments()
-                            .and_then(|segments| segments.last())
-                            .and_then(|name| if name.is_empty() { None } else { Some(name) })
-                            .and_then(|name| {
-                                PathBuf::from(name)
-                                    .extension()
-                                    .map(|name| name.to_string_lossy().to_string())
-                            });
-                        path_extension
-                    }
-                    _ => Some(content_type.subtype().to_string()),
-                };
-
-                let output = response_to_buffer(resp, engine_state, span);
-
-                if raw {
-                    return Ok(output);
-                }
-
-                if let Some(ext) = ext {
-                    match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
-                        Some(converter_id) => engine_state.get_decl(converter_id).run(
-                            engine_state,
-                            stack,
-                            &Call::new(span),
-                            output,
-                        ),
-                        None => Ok(output),
-                    }
-                } else {
-                    Ok(output)
-                }
-            }
-            None => Ok(response_to_buffer(resp, engine_state, span)),
-        },
-        Err(e) if e.is_timeout() => Err(ShellError::NetworkFailure(
-            format!("Request to {requested_url} has timed out"),
-            span,
-        )),
-        Err(e) if e.is_status() => match e.status() {
-            Some(err_code) if err_code == StatusCode::NOT_FOUND => Err(ShellError::NetworkFailure(
-                format!("Requested file not found (404): {requested_url:?}"),
-                span,
-            )),
-            Some(err_code) if err_code == StatusCode::MOVED_PERMANENTLY => {
-                Err(ShellError::NetworkFailure(
-                    format!("Resource moved permanently (301): {requested_url:?}"),
-                    span,
-                ))
-            }
-            Some(err_code) if err_code == StatusCode::BAD_REQUEST => Err(
-                ShellError::NetworkFailure(format!("Bad request (400) to {requested_url:?}"), span),
-            ),
-            Some(err_code) if err_code == StatusCode::FORBIDDEN => Err(ShellError::NetworkFailure(
-                format!("Access forbidden (403) to {requested_url:?}"),
-                span,
-            )),
-            _ => Err(ShellError::NetworkFailure(
-                format!(
-                    "Cannot make request to {:?}. Error is {:?}",
-                    requested_url,
-                    e.to_string()
-                ),
-                span,
-            )),
-        },
-        Err(e) => Err(ShellError::NetworkFailure(
-            format!(
-                "Cannot make request to {:?}. Error is {:?}",
-                requested_url,
-                e.to_string()
-            ),
-            span,
-        )),
-    }
-}
-
-fn response_to_buffer(
-    response: Response,
-    engine_state: &EngineState,
-    span: Span,
-) -> nu_protocol::PipelineData {
-    // Try to get the size of the file to be downloaded.
-    // This is helpful to show the progress of the stream.
-    let buffer_size = match &response.headers().get("content-length") {
-        Some(content_length) => {
-            let content_length = &(*content_length).clone(); // binding
-
-            let content_length = content_length
-                .to_str()
-                .unwrap_or("")
-                .parse::<u64>()
-                .unwrap_or(0);
-
-            if content_length == 0 {
-                None
-            } else {
-                Some(content_length)
-            }
-        }
-        _ => None,
-    };
-    let buffered_input = BufReader::new(response);
-
-    PipelineData::ExternalStream {
-        stdout: Some(RawStream::new(
-            Box::new(BufferedReader {
-                input: buffered_input,
-            }),
-            engine_state.ctrlc.clone(),
-            span,
-            buffer_size,
-        )),
-        stderr: None,
-        exit_code: None,
-        span,
-        metadata: None,
-        trim_end_newline: false,
-    }
+    let response = request.send().and_then(|r| r.error_for_status());
+    request_handle_response(engine_state, stack, span, &requested_url, raw, response)
 }

crates/nu-command/src/network/http/post.rs

@@ -1,20 +1,14 @@
-use crate::formats::value_to_json_value;
-use base64::{alphabet, engine::general_purpose::PAD, engine::GeneralPurpose, Engine};
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::util::BufferedReader;
-use nu_protocol::RawStream;
 use nu_protocol::{
-    Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value,
+    Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type, Value,
 };
-use reqwest::{blocking::Response, StatusCode};
-use std::collections::HashMap;
-use std::io::BufReader;
-use std::path::PathBuf;
-use std::str::FromStr;
 
-use crate::network::http::client::http_client;
+use crate::network::http::client::{
+    http_client, request_add_authorization_header, request_add_custom_headers,
+    request_handle_response, request_set_body, request_set_timeout,
+};
 
 #[derive(Clone)]
 pub struct SubCommand;
@@ -53,6 +47,12 @@ impl Command for SubCommand {
                 "the length of the content being posted",
                 Some('l'),
             )
+            .named(
+                "max-time",
+                SyntaxShape::Int,
+                "timeout period in seconds",
+                Some('m'),
+            )
             .named(
                 "headers",
                 SyntaxShape::Any,
@@ -124,6 +124,7 @@ impl Command for SubCommand {
 struct Arguments {
     path: Value,
     body: Value,
+    timeout: Option<Value>,
     headers: Option<Value>,
     raw: bool,
     insecure: Option<bool>,
@@ -133,13 +134,6 @@ struct Arguments {
     content_length: Option<String>,
 }
 
-#[derive(PartialEq, Eq)]
-enum BodyType {
-    Json,
-    Form,
-    Unknown,
-}
-
 fn run_post(
     engine_state: &EngineState,
     stack: &mut Stack,
@@ -149,6 +143,7 @@ fn run_post(
     let args = Arguments {
         path: call.req(engine_state, stack, 0)?,
         body: call.req(engine_state, stack, 1)?,
+        timeout: call.get_flag(engine_state, stack, "timeout")?,
         headers: call.get_flag(engine_state, stack, "headers")?,
         raw: call.has_flag("raw"),
         user: call.get_flag(engine_state, stack, "user")?,
@@ -159,6 +154,7 @@ fn run_post(
     };
     helper(engine_state, stack, call, args)
 }
+
 // Helper function that actually goes to retrieve the resource from the url given
 // The Option<String> return a possible file extension which can be used in AutoConvert commands
 fn helper(
@@ -166,9 +162,17 @@ fn helper(
     stack: &mut Stack,
     call: &Call,
     args: Arguments,
-) -> std::result::Result<PipelineData, ShellError> {
+) -> Result<PipelineData, ShellError> {
     let url_value = args.path;
     let body = args.body;
+    let user = args.user.clone();
+    let password = args.password;
+    let timeout = args.timeout;
+    let headers = args.headers;
+    let content_type = args.content_type;
+    let content_length = args.content_length;
+    let raw = args.raw;
+
     let span = url_value.span()?;
     let requested_url = url_value.as_string()?;
     let url = match url::Url::parse(&requested_url) {
@@ -183,253 +187,14 @@ fn helper(
             ));
         }
     };
-    let user = args.user.clone();
-    let password = args.password;
-    let headers = args.headers;
-    let location = url;
-    let raw = args.raw;
-    let base64_engine = GeneralPurpose::new(&alphabet::STANDARD, PAD);
 
-    let login = match (user, password) {
-        (Some(user), Some(password)) => {
-            let mut enc_str = String::new();
-            base64_engine.encode_string(&format!("{user}:{password}"), &mut enc_str);
-            Some(enc_str)
-        }
-        (Some(user), _) => {
-            let mut enc_str = String::new();
-            base64_engine.encode_string(&format!("{user}:"), &mut enc_str);
-            Some(enc_str)
-        }
-        _ => None,
-    };
+    let mut request = http_client(args.insecure.is_some()).post(url);
 
-    let body_type = match &args.content_type {
-        Some(it) if it == "application/json" => BodyType::Json,
-        Some(it) if it == "application/x-www-form-urlencoded" => BodyType::Form,
-        _ => BodyType::Unknown,
-    };
+    request = request_set_body(content_type, content_length, body, request)?;
+    request = request_set_timeout(timeout, request)?;
+    request = request_add_authorization_header(user, password, request);
+    request = request_add_custom_headers(headers, request)?;
 
-    let mut request = http_client(args.insecure.is_some()).post(location);
-
-    // set the content-type header before using e.g., request.json
-    // because that will avoid duplicating the header value
-    if let Some(val) = args.content_type {
-        request = request.header("Content-Type", val);
-    }
-
-    match body {
-        Value::Binary { val, .. } => {
-            request = request.body(val);
-        }
-        Value::String { val, .. } => {
-            request = request.body(val);
-        }
-        Value::Record { .. } if body_type == BodyType::Json => {
-            let data = value_to_json_value(&body)?;
-            request = request.json(&data);
-        }
-        Value::Record { .. } if body_type == BodyType::Form => {
-            let data = value_to_json_value(&body)?;
-            request = request.form(&data);
-        }
-        Value::List { vals, .. } if body_type == BodyType::Form => {
-            if vals.len() % 2 != 0 {
-                return Err(ShellError::IOError("unsupported body input".into()));
-            }
-            let data = vals
-                .chunks(2)
-                .map(|it| Ok((it[0].as_string()?, it[1].as_string()?)))
-                .collect::<Result<Vec<(String, String)>, ShellError>>()?;
-            request = request.form(&data)
-        }
-        _ => {
-            return Err(ShellError::IOError("unsupported body input".into()));
-        }
-    };
-
-    if let Some(val) = args.content_length {
-        request = request.header("Content-Length", val);
-    }
-    if let Some(login) = login {
-        request = request.header("Authorization", format!("Basic {login}"));
-    }
-
-    if let Some(headers) = headers {
-        let mut custom_headers: HashMap<String, Value> = HashMap::new();
-
-        match &headers {
-            Value::List { vals: table, .. } => {
-                if table.len() == 1 {
-                    // single row([key1 key2]; [val1 val2])
-                    match &table[0] {
-                        Value::Record { cols, vals, .. } => {
-                            for (k, v) in cols.iter().zip(vals.iter()) {
-                                custom_headers.insert(k.to_string(), v.clone());
-                            }
-                        }
-
-                        x => {
-                            return Err(ShellError::CantConvert(
-                                "string list or single row".into(),
-                                x.get_type().to_string(),
-                                headers.span().unwrap_or_else(|_| Span::new(0, 0)),
-                                None,
-                            ));
-                        }
-                    }
-                } else {
-                    // primitive values ([key1 val1 key2 val2])
-                    for row in table.chunks(2) {
-                        if row.len() == 2 {
-                            custom_headers.insert(row[0].as_string()?, row[1].clone());
-                        }
-                    }
-                }
-            }
-
-            x => {
-                return Err(ShellError::CantConvert(
-                    "string list or single row".into(),
-                    x.get_type().to_string(),
-                    headers.span().unwrap_or_else(|_| Span::new(0, 0)),
-                    None,
-                ));
-            }
-        };
-
-        for (k, v) in &custom_headers {
-            if let Ok(s) = v.as_string() {
-                request = request.header(k, s);
-            }
-        }
-    }
-
-    // Explicitly turn 4xx and 5xx statuses into errors.
-    match request.send().and_then(|r| r.error_for_status()) {
-        Ok(resp) => match resp.headers().get("content-type") {
-            Some(content_type) => {
-                let content_type = content_type.to_str().map_err(|e| {
-                    ShellError::GenericError(
-                        e.to_string(),
-                        "".to_string(),
-                        None,
-                        Some("MIME type were invalid".to_string()),
-                        Vec::new(),
-                    )
-                })?;
-                let content_type = mime::Mime::from_str(content_type).map_err(|_| {
-                    ShellError::GenericError(
-                        format!("MIME type unknown: {content_type}"),
-                        "".to_string(),
-                        None,
-                        Some("given unknown MIME type".to_string()),
-                        Vec::new(),
-                    )
-                })?;
-                let ext = match (content_type.type_(), content_type.subtype()) {
-                    (mime::TEXT, mime::PLAIN) => {
-                        let path_extension = url::Url::parse(&requested_url)
-                            .map_err(|_| {
-                                ShellError::GenericError(
-                                    format!("Cannot parse URL: {requested_url}"),
-                                    "".to_string(),
-                                    None,
-                                    Some("cannot parse".to_string()),
-                                    Vec::new(),
-                                )
-                            })?
-                            .path_segments()
-                            .and_then(|segments| segments.last())
-                            .and_then(|name| if name.is_empty() { None } else { Some(name) })
-                            .and_then(|name| {
-                                PathBuf::from(name)
-                                    .extension()
-                                    .map(|name| name.to_string_lossy().to_string())
-                            });
-                        path_extension
-                    }
-                    _ => Some(content_type.subtype().to_string()),
-                };
-                let output = response_to_buffer(resp, engine_state, span);
-
-                if raw {
-                    return Ok(output);
-                }
-                if let Some(ext) = ext {
-                    match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
-                        Some(converter_id) => engine_state.get_decl(converter_id).run(
-                            engine_state,
-                            stack,
-                            &Call::new(span),
-                            output,
-                        ),
-                        None => Ok(output),
-                    }
-                } else {
-                    Ok(output)
-                }
-            }
-            None => Ok(response_to_buffer(resp, engine_state, span)),
-        },
-        Err(e) if e.is_status() => match e.status() {
-            Some(err_code) if err_code == StatusCode::NOT_FOUND => Err(ShellError::NetworkFailure(
-                format!("Requested file not found (404): {requested_url:?}"),
-                span,
-            )),
-            Some(err_code) if err_code == StatusCode::MOVED_PERMANENTLY => {
-                Err(ShellError::NetworkFailure(
-                    format!("Resource moved permanently (301): {requested_url:?}"),
-                    span,
-                ))
-            }
-            Some(err_code) if err_code == StatusCode::BAD_REQUEST => Err(
-                ShellError::NetworkFailure(format!("Bad request (400) to {requested_url:?}"), span),
-            ),
-            Some(err_code) if err_code == StatusCode::FORBIDDEN => Err(ShellError::NetworkFailure(
-                format!("Access forbidden (403) to {requested_url:?}"),
-                span,
-            )),
-            _ => Err(ShellError::NetworkFailure(
-                format!(
-                    "Cannot make request to {:?}. Error is {:?}",
-                    requested_url,
-                    e.to_string()
-                ),
-                span,
-            )),
-        },
-        Err(e) => Err(ShellError::NetworkFailure(
-            format!(
-                "Cannot make request to {:?}. Error is {:?}",
-                requested_url,
-                e.to_string()
-            ),
-            span,
-        )),
-    }
-}
-
-fn response_to_buffer(
-    response: Response,
-    engine_state: &EngineState,
-    span: Span,
-) -> nu_protocol::PipelineData {
-    let buffered_input = BufReader::new(response);
-
-    PipelineData::ExternalStream {
-        stdout: Some(RawStream::new(
-            Box::new(BufferedReader {
-                input: buffered_input,
-            }),
-            engine_state.ctrlc.clone(),
-            span,
-            None,
-        )),
-        stderr: None,
-        exit_code: None,
-        span,
-        metadata: None,
-        trim_end_newline: false,
-    }
+    let response = request.send().and_then(|r| r.error_for_status());
+    request_handle_response(engine_state, stack, span, &requested_url, raw, response)
 }