Don't quote string values in query parameters (#747)

* Read params regardless of input presence

Currently we only check for params added via `-f/--raw-field` and
`-F/--field` when `self.input` is not empty. It is valid to pass
parameters when no body has been set, set params independently of
input.

* Don't quote string values in query parameters

We read string values into a `serde_json::Value::String` when parsing
parameters, using its `Display` format when appending `{key}={value}`
to the URI. This is incorrect as `Value::String` encloses the value with
double quotes[0].

Use the wrapped string for `Value::String` values to avoid wrapping the
string in quotes and reject `Value::Array` and `Value::Object` values as
unrepresentable.

Full URI before:

  https://oxide.sys.example.com/v1/instances?project=\"will\"

Full URI after:

  https://oxide.sys.example.com/v1/instances?project=will

[0] https://github.com/serde-rs/json/blob/3f1c6de4af28b1f6c5100da323f2bffaf7c2083f/README.md#L121-L127

Author: wfchandler

cli/src/cmd_api.rs

@@ -9,12 +9,13 @@ use std::{
     io::{Read, Write},
 };
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, bail, Result};
 use async_trait::async_trait;
 use clap::Parser;
 use futures::{StreamExt, TryStreamExt};
 use oxide::Client;
 use serde::Deserialize;
+use serde_json::Value;
 
 use crate::{print_nopipe, println_nopipe};
 
@@ -150,19 +151,27 @@ impl crate::AuthenticatedCmd for CmdApi {
 
             // Set this as our body.
             bytes = buf.clone();
+        }
 
-            // Set our params to the query string.
-            if !params.is_empty() {
-                let mut query_string = String::new();
-                for (key, value) in params {
-                    if !query_string.is_empty() {
-                        query_string.push('&');
+        // Set our params to the query string.
+        if !params.is_empty() {
+            let mut query_string = String::new();
+            for (key, value) in params {
+                if !query_string.is_empty() {
+                    query_string.push('&');
+                }
+                match &value {
+                    Value::String(s) => query_string.push_str(&format!("{key}={s}")),
+                    Value::Bool(b) => query_string.push_str(&format!("{key}={b}")),
+                    Value::Number(n) => query_string.push_str(&format!("{key}={n}")),
+                    Value::Null => query_string.push_str(&format!("{key}=null")),
+                    Value::Array(_) | Value::Object(_) => {
+                        bail!("invalid query parameter value {}", value)
                     }
-                    query_string.push_str(&format!("{}={}", key, value));
                 }
-
-                endpoint_with_query = add_query_string(&endpoint_with_query, &query_string);
             }
+
+            endpoint_with_query = add_query_string(&endpoint_with_query, &query_string);
         }
 
         let rclient = client.client();

cli/tests/test_api.rs

@@ -10,7 +10,8 @@ use predicates::prelude::*;
 use serde::Serialize;
 use serde_json::json;
 
-/// Validate `oxide api` for a simple GET with parameters.
+/// Validate `oxide api` for a simple GET with parameters embedded in URI
+/// and via the `--raw-field` option.
 #[test]
 fn test_simple_api_call() {
     let server = MockServer::start();
@@ -37,7 +38,23 @@ fn test_simple_api_call() {
         .success()
         .stdout(predicate::str::diff("{\n  \"a\": \"b\"\n}\n"));
 
-    mock.assert();
+    Command::cargo_bin("oxide")
+        .unwrap()
+        .env("OXIDE_HOST", server.url(""))
+        .env("OXIDE_TOKEN", "fake-token")
+        .arg("api")
+        .arg("-X")
+        .arg("GET")
+        .arg("/simple/test/call")
+        .arg("--raw-field")
+        .arg("param1=value1")
+        .arg("-f")
+        .arg("param2=value2")
+        .assert()
+        .success()
+        .stdout(predicate::str::diff("{\n  \"a\": \"b\"\n}\n"));
+
+    mock.assert_hits(2);
 }
 
 #[derive(Serialize)]