would like support for freeform request bodies (fixes oxidecomputer#44)

dropshot/src/api_description.rs

@@ -12,6 +12,7 @@ use crate::router::HttpRouter;
 use crate::router::PathSegment;
 use crate::Extractor;
 use crate::CONTENT_TYPE_JSON;
+use crate::CONTENT_TYPE_OCTET_STREAM;
 
 use http::Method;
 use http::StatusCode;
@@ -77,7 +78,7 @@ impl<'a> ApiEndpoint {
  */
 #[derive(Debug)]
 pub struct ApiEndpointParameter {
-    pub name: ApiEndpointParameterName,
+    pub metadata: ApiEndpointParameterMetadata,
     pub description: Option<String>,
     pub required: bool,
     pub schema: ApiSchemaGenerator,
@@ -94,12 +95,12 @@ impl ApiEndpointParameter {
         examples: Vec<String>,
     ) -> Self {
         Self {
-            name: match loc {
+            metadata: match loc {
                 ApiEndpointParameterLocation::Path => {
-                    ApiEndpointParameterName::Path(name)
+                    ApiEndpointParameterMetadata::Path(name)
                 }
                 ApiEndpointParameterLocation::Query => {
-                    ApiEndpointParameterName::Query(name)
+                    ApiEndpointParameterMetadata::Query(name)
                 }
             },
             description,
@@ -110,13 +111,14 @@ impl ApiEndpointParameter {
     }
 
     pub fn new_body(
+        content_type: ApiEndpointBodyContentType,
         description: Option<String>,
         required: bool,
         schema: ApiSchemaGenerator,
         examples: Vec<String>,
     ) -> Self {
         Self {
-            name: ApiEndpointParameterName::Body,
+            metadata: ApiEndpointParameterMetadata::Body(content_type),
             description,
             required,
             schema,
@@ -132,10 +134,27 @@ pub enum ApiEndpointParameterLocation {
 }
 
 #[derive(Debug, Clone)]
-pub enum ApiEndpointParameterName {
+pub enum ApiEndpointParameterMetadata {
     Path(String),
     Query(String),
-    Body,
+    Body(ApiEndpointBodyContentType),
+}
+
+#[derive(Debug, Clone)]
+pub enum ApiEndpointBodyContentType {
+    /** application/octet-stream */
+    Bytes,
+    /** application/json */
+    Json,
+}
+
+impl ApiEndpointBodyContentType {
+    fn mime_type(&self) -> &str {
+        match self {
+            ApiEndpointBodyContentType::Bytes => CONTENT_TYPE_OCTET_STREAM,
+            ApiEndpointBodyContentType::Json => CONTENT_TYPE_JSON,
+        }
+    }
 }
 
 /**
@@ -211,8 +230,8 @@ impl ApiDescription {
         let vars = e
             .parameters
             .iter()
-            .filter_map(|p| match &p.name {
-                ApiEndpointParameterName::Path(name) => Some(name.clone()),
+            .filter_map(|p| match &p.metadata {
+                ApiEndpointParameterMetadata::Path(name) => Some(name.clone()),
                 _ => None,
             })
             .collect::<HashSet<_>>();
@@ -247,6 +266,23 @@ impl ApiDescription {
             };
         }
 
+        // Explicitly disallow any attempt to consume the body twice.
+        let nbodyextractors = e
+            .parameters
+            .iter()
+            .filter(|p| match p.metadata {
+                ApiEndpointParameterMetadata::Body(..) => true,
+                _ => false,
+            })
+            .count();
+        if nbodyextractors > 1 {
+            return Err(format!(
+                "only one body extractor can be used in a handler (this \
+                 function has {})",
+                nbodyextractors
+            ));
+        }
+
         self.router.insert(e);
 
         Ok(())
@@ -360,12 +396,12 @@ impl ApiDescription {
                 .parameters
                 .iter()
                 .filter_map(|param| {
-                    let (name, location) = match &param.name {
-                        ApiEndpointParameterName::Body => return None,
-                        ApiEndpointParameterName::Path(name) => {
+                    let (name, location) = match &param.metadata {
+                        ApiEndpointParameterMetadata::Body(_) => return None,
+                        ApiEndpointParameterMetadata::Path(name) => {
                             (name, ApiEndpointParameterLocation::Path)
                         }
-                        ApiEndpointParameterName::Query(name) => {
+                        ApiEndpointParameterMetadata::Query(name) => {
                             (name, ApiEndpointParameterLocation::Query)
                         }
                     };
@@ -417,10 +453,12 @@ impl ApiDescription {
                 .parameters
                 .iter()
                 .filter_map(|param| {
-                    match &param.name {
-                        ApiEndpointParameterName::Body => (),
+                    let mime_type = match &param.metadata {
+                        ApiEndpointParameterMetadata::Body(ct) => {
+                            ct.mime_type()
+                        }
                         _ => return None,
-                    }
+                    };
 
                     let (name, js) = match &param.schema {
                         ApiSchemaGenerator::Gen {
@@ -435,7 +473,7 @@ impl ApiDescription {
 
                     let mut content = indexmap::IndexMap::new();
                     content.insert(
-                        CONTENT_TYPE_JSON.to_string(),
+                        mime_type.to_string(),
                         openapiv3::MediaType {
                             schema: Some(schema),
                             example: None,
@@ -1073,6 +1111,10 @@ mod test {
     use super::j2oas_schema;
     use super::ApiDescription;
     use super::ApiEndpoint;
+    use crate as dropshot; /* for "endpoint" macro */
+    use crate::endpoint;
+    use crate::TypedBody;
+    use crate::UntypedBody;
     use http::Method;
     use hyper::Body;
     use hyper::Response;
@@ -1194,4 +1236,30 @@ mod test {
             let _ = j2oas_schema(Some(key), schema);
         }
     }
+
+    #[test]
+    fn test_two_bodies() {
+        #[derive(Deserialize, JsonSchema)]
+        struct AStruct {};
+
+        #[endpoint {
+            method = PUT,
+            path = "/testing/two_bodies"
+        }]
+        async fn test_twobodies_handler(
+            _: Arc<RequestContext>,
+            _: UntypedBody,
+            _: TypedBody<AStruct>,
+        ) -> Result<Response<Body>, HttpError> {
+            unimplemented!();
+        }
+
+        let mut api = ApiDescription::new();
+        let error = api.register(test_twobodies_handler).unwrap_err();
+        assert_eq!(
+            error,
+            "only one body extractor can be used in a handler (this function \
+             has 2)"
+        );
+    }
 }

dropshot/src/handler.rs

@@ -38,18 +38,22 @@ use super::http_util::http_extract_path_params;
 use super::http_util::http_read_body;
 use super::http_util::CONTENT_TYPE_JSON;
 use super::server::DropshotState;
+use crate::api_description::ApiEndpointBodyContentType;
 use crate::api_description::ApiEndpointParameter;
 use crate::api_description::ApiEndpointParameterLocation;
 use crate::api_description::ApiEndpointResponse;
 use crate::api_description::ApiSchemaGenerator;
 use crate::pagination::PaginationParams;
 
 use async_trait::async_trait;
+use bytes::Bytes;
 use futures::lock::Mutex;
 use http::StatusCode;
 use hyper::Body;
 use hyper::Request;
 use hyper::Response;
+use schemars::schema::InstanceType;
+use schemars::schema::SchemaObject;
 use schemars::JsonSchema;
 use serde::de::DeserializeOwned;
 use serde::Serialize;
@@ -155,11 +159,11 @@ impl RequestContext {
  * `RequestContext`.  Unlike most traits, `Extractor` essentially defines only a
  * constructor function, not instance functions.
  *
- * The extractors that we provide (`Query`, `Path`, `TypedBody`) implement
- * `Extractor` in order to construct themselves from the request. For example,
- * `Extractor` is implemented for `Query<Q>` with a function that reads the
- * query string from the request, parses it, and constructs a `Query<Q>` with
- * it.
+ * The extractors that we provide (`Query`, `Path`, `TypedBody`, and
+ * `UntypedBody`) implement `Extractor` in order to construct themselves from
+ * the request. For example, `Extractor` is implemented for `Query<Q>` with a
+ * function that reads the query string from the request, parses it, and
+ * constructs a `Query<Q>` with it.
  *
  * We also define implementations of `Extractor` for tuples of types that
  * themselves implement `Extractor`.  See the implementation of
@@ -805,7 +809,8 @@ fn schema2parameters(
 }
 
 /*
- * JSON: json body extractor
+ * TypedBody: body extractor for formats that can be deserialized to a specific
+ * type.  Only JSON is currently supported.
  */
 
 /**
@@ -880,6 +885,7 @@ where
 
     fn metadata() -> Vec<ApiEndpointParameter> {
         vec![ApiEndpointParameter::new_body(
+            ApiEndpointBodyContentType::Json,
             None,
             true,
             ApiSchemaGenerator::Gen {
@@ -891,6 +897,78 @@ where
     }
 }
 
+/*
+ * UntypedBody: body extractor for a plain array of bytes of a body.
+ */
+
+/**
+ * `UntypedBody` is an extractor for reading in the contents of the HTTP request
+ * body and making the raw bytes directly available to the consumer.
+ */
+pub struct UntypedBody {
+    content: Bytes,
+}
+
+impl UntypedBody {
+    /**
+     * Returns a byte slice of the underlying body content.
+     */
+    /*
+     * TODO drop this in favor of Deref?  + Display and Debug for convenience?
+     */
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.content
+    }
+
+    /**
+     * Convenience wrapper to convert the body to a UTF-8 string slice,
+     * returning a 400-level error if the body is not valid UTF-8.
+     */
+    pub fn as_str(&self) -> Result<&str, HttpError> {
+        std::str::from_utf8(self.as_bytes()).map_err(|e| {
+            HttpError::for_bad_request(
+                None,
+                format!("failed to parse body as UTF-8 string: {}", e),
+            )
+        })
+    }
+}
+
+#[async_trait]
+impl Extractor for UntypedBody {
+    async fn from_request(
+        rqctx: Arc<RequestContext>,
+    ) -> Result<UntypedBody, HttpError> {
+        let server = &rqctx.server;
+        let mut request = rqctx.request.lock().await;
+        let body_bytes = http_read_body(
+            request.body_mut(),
+            server.config.request_body_max_bytes,
+        )
+        .await?;
+        Ok(UntypedBody {
+            content: body_bytes,
+        })
+    }
+
+    fn metadata() -> Vec<ApiEndpointParameter> {
+        let schema = SchemaObject {
+            instance_type: Some(InstanceType::String.into()),
+            format: Some(String::from("binary")),
+            ..Default::default()
+        }
+        .into();
+
+        vec![ApiEndpointParameter::new_body(
+            ApiEndpointBodyContentType::Bytes,
+            None,
+            true,
+            ApiSchemaGenerator::Static(schema),
+            vec![],
+        )]
+    }
+}
+
 /*
  * Response Type Conversion
  *
@@ -1107,7 +1185,7 @@ impl From<HttpResponseUpdatedNoContent> for HttpHandlerResult {
 mod test {
     use super::GetMetadata;
     use crate::{
-        api_description::ApiEndpointParameterName, ApiEndpointParameter,
+        api_description::ApiEndpointParameterMetadata, ApiEndpointParameter,
         ApiEndpointParameterLocation, PaginationParams,
     };
     use schemars::JsonSchema;
@@ -1146,7 +1224,7 @@ mod test {
         actual.iter().zip(expected.iter()).for_each(
             |(param, (name, required))| {
                 if let ApiEndpointParameter {
-                    name: ApiEndpointParameterName::Path(aname),
+                    metadata: ApiEndpointParameterMetadata::Path(aname),
                     required: arequired,
                     ..
                 } = param

dropshot/src/http_util.rs

@@ -14,6 +14,8 @@ use crate::from_map::from_map;
 
 /** header name for conveying request ids ("x-request-id") */
 pub const HEADER_REQUEST_ID: &str = "x-request-id";
+/** MIME type for raw bytes */
+pub const CONTENT_TYPE_OCTET_STREAM: &str = "application/octet-stream";
 /** MIME type for plain JSON data */
 pub const CONTENT_TYPE_JSON: &str = "application/json";
 /** MIME type for newline-delimited JSON data */
@@ -41,6 +43,7 @@ where
      * std::marker::Unpin, &mut T)
      * TODO Error type shouldn't have to be hyper Error -- Into<ApiError> should
      * work too?
+     * TODO do we need to use saturating_add() here?
      */
     let mut parts = std::vec::Vec::new();
     let mut nbytesread: usize = 0;
@@ -90,6 +93,7 @@ where
      * read?  What if the underlying implementation chooses to wait for a much
      * larger number of bytes?
      * TODO better understand pin_mut!()
+     * TODO do we need to use saturating_add() here?
      */
     let mut nbytesread: usize = 0;
     while let Some(maybebuf) = body.data().await {