From f9d92025651aa94613f89ec1b2df5802e4eba5a6 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 15:44:27 -0700 Subject: [PATCH 1/9] add response types for various 300-level response codes --- dropshot/src/handler.rs | 95 ++++++++++++++++++++++++++++++ dropshot/src/lib.rs | 3 + dropshot/tests/test_demo.rs | 114 ++++++++++++++++++++++++++++++++++++ 3 files changed, 212 insertions(+) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 5f7575ec6..d4f2da682 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1436,6 +1436,101 @@ impl From for HttpHandlerResult { } } +/** + * `HttpResponseFoundNoContent` represents an HTTP 302 "Found" response with no + * response body. + * + * The sole item, a `String`, will become the value of the `Location` header. + * This is where you want to redirect the client to. + * + * Per MDN and RFC 9110 S15.4.3, you might want to use 307 ("Temporary + * Redirect") or 303 ("See Other") instead. + */ +pub struct HttpResponseFoundNoContent(pub String); + +impl HttpCodedResponse for HttpResponseFoundNoContent { + type Body = Empty; + const STATUS_CODE: StatusCode = StatusCode::FOUND; + const DESCRIPTION: &'static str = "redirect (302)"; +} + +impl From for HttpHandlerResult { + fn from(response: HttpResponseFoundNoContent) -> HttpHandlerResult { + const STATUS_CODE: http::StatusCode = + ::STATUS_CODE; + Empty.to_response( + Response::builder() + .status(STATUS_CODE) + .header(http::header::LOCATION, &response.0), + ) + } +} + +/** + * `HttpResponseSeeOtherNoContent` represents an HTTP 303 "See Other" response + * with no response body. + * + * The sole item, a `String`, will become the value of the `Location` header. + * This is where you want to redirect the client to. + * + * Use this (as opposed to 307 "Temporary Redirect") when you want the client to + * follow up with a GET, rather than whatever method they used to make the + * current request. This is intended to be used after a PUT or POST to show a + * confirmation page or the like. + */ +pub struct HttpResponseSeeOtherNoContent(pub String); + +impl HttpCodedResponse for HttpResponseSeeOtherNoContent { + type Body = Empty; + const STATUS_CODE: StatusCode = StatusCode::SEE_OTHER; + const DESCRIPTION: &'static str = "redirect (303)"; +} + +impl From for HttpHandlerResult { + fn from(response: HttpResponseSeeOtherNoContent) -> HttpHandlerResult { + const STATUS_CODE: http::StatusCode = + ::STATUS_CODE; + Empty.to_response( + Response::builder() + .status(STATUS_CODE) + .header(http::header::LOCATION, &response.0), + ) + } +} + +/** + * `HttpResponseTemporaryRedirectNoContent` represents an HTTP 307 "Temporary + * Redirect" response with no response body. + * + * The sole item, a `String`, will become the value of the `Location` header. + * This is where you want to redirect the client to. + * + * Use this (as opposed to 303 "See Other") when you want the client to use the + * same request method and body when it makes the follow-up request. + */ +pub struct HttpResponseTemporaryRedirectNoContent(pub String); + +impl HttpCodedResponse for HttpResponseTemporaryRedirectNoContent { + type Body = Empty; + const STATUS_CODE: StatusCode = StatusCode::SEE_OTHER; + const DESCRIPTION: &'static str = "redirect (307)"; +} + +impl From for HttpHandlerResult { + fn from( + response: HttpResponseTemporaryRedirectNoContent, + ) -> HttpHandlerResult { + const STATUS_CODE: http::StatusCode = + ::STATUS_CODE; + Empty.to_response( + Response::builder() + .status(STATUS_CODE) + .header(http::header::LOCATION, &response.0), + ) + } +} + #[derive(Serialize, JsonSchema)] pub struct NoHeaders {} diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index 6d29c4d37..3d672bc64 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -642,8 +642,11 @@ pub use handler::HttpResponse; pub use handler::HttpResponseAccepted; pub use handler::HttpResponseCreated; pub use handler::HttpResponseDeleted; +pub use handler::HttpResponseFoundNoContent; pub use handler::HttpResponseHeaders; pub use handler::HttpResponseOk; +pub use handler::HttpResponseSeeOtherNoContent; +pub use handler::HttpResponseTemporaryRedirectNoContent; pub use handler::HttpResponseUpdatedNoContent; pub use handler::NoHeaders; pub use handler::Path; diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index 5fd57c26b..cca857b69 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -25,8 +25,11 @@ use dropshot::test_util::TEST_HEADER_2; use dropshot::ApiDescription; use dropshot::HttpError; use dropshot::HttpResponseDeleted; +use dropshot::HttpResponseFoundNoContent; use dropshot::HttpResponseHeaders; use dropshot::HttpResponseOk; +use dropshot::HttpResponseSeeOtherNoContent; +use dropshot::HttpResponseTemporaryRedirectNoContent; use dropshot::HttpResponseUpdatedNoContent; use dropshot::Path; use dropshot::Query; @@ -68,6 +71,9 @@ fn demo_api() -> ApiDescription { api.register(demo_handler_untyped_body).unwrap(); api.register(demo_handler_delete).unwrap(); api.register(demo_handler_headers).unwrap(); + api.register(demo_handler_302_found).unwrap(); + api.register(demo_handler_303_see_other).unwrap(); + api.register(demo_handler_307_temporary_redirect).unwrap(); api.register(demo_handler_websocket).unwrap(); /* @@ -743,6 +749,84 @@ async fn test_header_request() { assert_eq!(headers, vec!["hi", "howdy"]); } +/* + * Test 302 "Found" response + */ +#[tokio::test] +async fn test_302_found() { + let api = demo_api(); + let testctx = common::test_setup("test_302_found", api); + let response = testctx + .client_testctx + .make_request( + Method::GET, + "/testing/test_302_found", + None as Option<()>, + StatusCode::FOUND, + ) + .await + .expect("expected success"); + let headers = response + .headers() + .get_all(http::header::LOCATION) + .iter() + .map(|v| v.to_str().unwrap()) + .collect::>(); + assert_eq!(headers, vec!["/path1"]); +} + +/* + * Test 303 "See Other" response + */ +#[tokio::test] +async fn test_303_see_other() { + let api = demo_api(); + let testctx = common::test_setup("test_303_see_other", api); + let response = testctx + .client_testctx + .make_request( + Method::GET, + "/testing/test_303_see_other", + None as Option<()>, + StatusCode::SEE_OTHER, + ) + .await + .expect("expected success"); + let headers = response + .headers() + .get_all(http::header::LOCATION) + .iter() + .map(|v| v.to_str().unwrap()) + .collect::>(); + assert_eq!(headers, vec!["/path2"]); +} + +/* + * Test 307 "Temporary Redirect" response + */ +#[tokio::test] +async fn test_307_temporary_redirect() { + let api = demo_api(); + let testctx = common::test_setup("test_307_temporary_redirect", api); + let response = testctx + .client_testctx + .make_request( + Method::GET, + "/testing/test_307_temporary_redirect", + None as Option<()>, + StatusCode::TEMPORARY_REDIRECT, + ) + .await + .expect("expected success"); + let headers = response + .headers() + .get_all(http::header::LOCATION) + .iter() + .map(|v| v.to_str().unwrap()) + .collect::>(); + assert_eq!(headers, vec!["/path3"]); +} + /* * The "test_demo_websocket" handler upgrades to a websocket and exchanges * greetings with the client. @@ -964,6 +1048,36 @@ async fn demo_handler_headers( Ok(response) } +#[endpoint { + method = GET, + path = "/testing/302_found", +}] +async fn demo_handler_302_found( + _rqctx: RequestCtx, +) -> Result { + Ok(HttpResponseFoundNoContent(String::from("/path1"))) +} + +#[endpoint { + method = GET, + path = "/testing/303_see_other", +}] +async fn demo_handler_303_see_other( + _rqctx: RequestCtx, +) -> Result { + Ok(HttpResponseSeeOtherNoContent(String::from("/path2"))) +} + +#[endpoint { + method = GET, + path = "/testing/307_temporary_redirect", +}] +async fn demo_handler_307_temporary_redirect( + _rqctx: RequestCtx, +) -> Result { + Ok(HttpResponseTemporaryRedirectNoContent(String::from("/path3"))) +} + #[channel { protocol = WEBSOCKETS, path = "/testing/websocket" From dd8a4b794dfc136443aad0c0e0de591354ffaf3c Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 15:56:33 -0700 Subject: [PATCH 2/9] more fixes --- CHANGELOG.adoc | 2 + dropshot/src/handler.rs | 2 +- dropshot/src/test_util.rs | 3 +- dropshot/tests/fail/bad_endpoint12.stderr | 3 ++ dropshot/tests/test_demo.rs | 6 +-- dropshot/tests/test_openapi.json | 57 +++++++++++++++++++++++ dropshot/tests/test_openapi.rs | 45 ++++++++++++++++-- dropshot/tests/test_openapi_fuller.json | 57 +++++++++++++++++++++++ 8 files changed, 166 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index fb73b54ea..c6934a32b 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,8 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] +* https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See [`HttpResponseFoundNoContent`], [`HttpResponseSeeOtherNoContent`], and [`HttpResponseTemporaryRedirectNoContent`]. + == 0.8.0 (released 2022-09-09) https://github.com/oxidecomputer/dropshot/compare/v0.7.0\...v0.8.0[Full list of commits] diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index d4f2da682..6b3f8a51f 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1512,7 +1512,7 @@ pub struct HttpResponseTemporaryRedirectNoContent(pub String); impl HttpCodedResponse for HttpResponseTemporaryRedirectNoContent { type Body = Empty; - const STATUS_CODE: StatusCode = StatusCode::SEE_OTHER; + const STATUS_CODE: StatusCode = StatusCode::TEMPORARY_REDIRECT; const DESCRIPTION: &'static str = "redirect (307)"; } diff --git a/dropshot/src/test_util.rs b/dropshot/src/test_util.rs index 12a80cf4f..7271bae77 100644 --- a/dropshot/src/test_util.rs +++ b/dropshot/src/test_util.rs @@ -57,10 +57,11 @@ pub const TEST_HEADER_2: &str = "x-dropshot-test-header-2"; // List of allowed HTTP headers in responses. // Used to make sure we don't leak headers unexpectedly. -const ALLOWED_HEADERS: [AllowedHeader<'static>; 7] = [ +const ALLOWED_HEADERS: [AllowedHeader<'static>; 8] = [ AllowedHeader::new("content-length"), AllowedHeader::new("content-type"), AllowedHeader::new("date"), + AllowedHeader::new("location"), AllowedHeader::new("x-request-id"), AllowedHeader { name: "transfer-encoding", diff --git a/dropshot/tests/fail/bad_endpoint12.stderr b/dropshot/tests/fail/bad_endpoint12.stderr index b352bb4de..f19d27868 100644 --- a/dropshot/tests/fail/bad_endpoint12.stderr +++ b/dropshot/tests/fail/bad_endpoint12.stderr @@ -8,7 +8,10 @@ error[E0277]: the trait bound `String: HttpCodedResponse` is not satisfied HttpResponseAccepted HttpResponseCreated HttpResponseDeleted + HttpResponseFoundNoContent HttpResponseOk + HttpResponseSeeOtherNoContent + HttpResponseTemporaryRedirectNoContent HttpResponseUpdatedNoContent = note: required because of the requirements on the impl of `HttpResponse` for `String` note: required because of the requirements on the impl of `ResultTrait` for `Result` diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index cca857b69..c14ba6aa3 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -760,7 +760,7 @@ async fn test_302_found() { .client_testctx .make_request( Method::GET, - "/testing/test_302_found", + "/testing/302_found", None as Option<()>, StatusCode::FOUND, ) @@ -786,7 +786,7 @@ async fn test_303_see_other() { .client_testctx .make_request( Method::GET, - "/testing/test_303_see_other", + "/testing/303_see_other", None as Option<()>, StatusCode::SEE_OTHER, ) @@ -812,7 +812,7 @@ async fn test_307_temporary_redirect() { .client_testctx .make_request( Method::GET, - "/testing/test_307_temporary_redirect", + "/testing/307_temporary_redirect", None as Option<()>, StatusCode::TEMPORARY_REDIRECT, ) diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index c164672de..80f50e2b2 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -285,6 +285,63 @@ } } }, + "/test/302_found": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler21", + "responses": { + "302": { + "description": "redirect (302)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/test/303_see_other": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler22", + "responses": { + "303": { + "description": "redirect (303)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/test/307_temporary_redirect": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler23", + "responses": { + "307": { + "description": "redirect (307)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/test/camera": { "post": { "tags": [ diff --git a/dropshot/tests/test_openapi.rs b/dropshot/tests/test_openapi.rs index 2ecaa39f8..defa331ca 100644 --- a/dropshot/tests/test_openapi.rs +++ b/dropshot/tests/test_openapi.rs @@ -2,10 +2,11 @@ use dropshot::{ endpoint, ApiDescription, FreeformBody, HttpError, HttpResponseAccepted, - HttpResponseCreated, HttpResponseDeleted, HttpResponseHeaders, - HttpResponseOk, HttpResponseUpdatedNoContent, PaginationParams, Path, - Query, RequestContext, ResultsPage, TagConfig, TagDetails, TypedBody, - UntypedBody, + HttpResponseCreated, HttpResponseDeleted, HttpResponseFoundNoContent, + HttpResponseHeaders, HttpResponseOk, HttpResponseSeeOtherNoContent, + HttpResponseTemporaryRedirectNoContent, HttpResponseUpdatedNoContent, + PaginationParams, Path, Query, RequestContext, ResultsPage, TagConfig, + TagDetails, TypedBody, UntypedBody, }; use hyper::Body; use schemars::JsonSchema; @@ -420,6 +421,39 @@ async fn handler20( Ok(HttpResponseCreated(Response {})) } +#[endpoint { + method = GET, + path = "/test/302_found", + tags = [ "it"], +}] +async fn handler21( + _rqctx: Arc>, +) -> Result { + Ok(HttpResponseFoundNoContent(String::from("/path1"))) +} + +#[endpoint { + method = GET, + path = "/test/303_see_other", + tags = [ "it"], +}] +async fn handler22( + _rqctx: Arc>, +) -> Result { + Ok(HttpResponseSeeOtherNoContent(String::from("/path2"))) +} + +#[endpoint { + method = GET, + path = "/test/307_temporary_redirect", + tags = [ "it"], +}] +async fn handler23( + _rqctx: Arc>, +) -> Result { + Ok(HttpResponseTemporaryRedirectNoContent(String::from("/path3"))) +} + fn make_api( maybe_tag_config: Option, ) -> Result, String> { @@ -449,6 +483,9 @@ fn make_api( api.register(handler18)?; api.register(handler19)?; api.register(handler20)?; + api.register(handler21)?; + api.register(handler22)?; + api.register(handler23)?; Ok(api) } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 1f141dbc2..bae91e03a 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -293,6 +293,63 @@ } } }, + "/test/302_found": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler21", + "responses": { + "302": { + "description": "redirect (302)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/test/303_see_other": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler22", + "responses": { + "303": { + "description": "redirect (303)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/test/307_temporary_redirect": { + "get": { + "tags": [ + "it" + ], + "operationId": "handler23", + "responses": { + "307": { + "description": "redirect (307)" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/test/camera": { "post": { "tags": [ From d871624d5709ef2444e2b2da43d5b95db112289d Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 15:57:20 -0700 Subject: [PATCH 3/9] fix changelog render --- CHANGELOG.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index c6934a32b..02630bc9e 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,7 +15,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] -* https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See [`HttpResponseFoundNoContent`], [`HttpResponseSeeOtherNoContent`], and [`HttpResponseTemporaryRedirectNoContent`]. +* https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFoundNoContent`, `HttpResponseSeeOtherNoContent`, and `HttpResponseTemporaryRedirectNoContent`. == 0.8.0 (released 2022-09-09) From 283eca60adeeaa06a2341d04c6988e07ea0c4033 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 17:19:39 -0700 Subject: [PATCH 4/9] abandoned attempt to encode headers as Vec --- CHANGELOG.adoc | 2 +- dropshot/src/handler.rs | 190 ++++++++++++++++------ dropshot/src/lib.rs | 9 +- dropshot/tests/fail/bad_endpoint12.stderr | 6 +- dropshot/tests/fail/bad_endpoint4.stderr | 2 +- dropshot/tests/fail/bad_endpoint7.stderr | 2 +- dropshot/tests/test_demo.rs | 23 ++- dropshot/tests/test_openapi.json | 39 ++++- dropshot/tests/test_openapi.rs | 27 +-- dropshot/tests/test_openapi_fuller.json | 39 ++++- 10 files changed, 253 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 02630bc9e..422fe43b6 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,7 +15,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.8.0\...HEAD[Full list of commits] -* https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFoundNoContent`, `HttpResponseSeeOtherNoContent`, and `HttpResponseTemporaryRedirectNoContent`. +* https://github.com/oxidecomputer/dropshot/pull/451[#451] There are now response types to support 302 ("Found"), 303 ("See Other"), and 307 ("Temporary Redirect") HTTP response codes. See `HttpResponseFound`, `HttpResponseSeeOther`, and `HttpResponseTemporaryRedirect`. == 0.8.0 (released 2022-09-09) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 6b3f8a51f..fa268345b 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1436,98 +1436,188 @@ impl From for HttpHandlerResult { } } +/** Describes headers associated with a 300-level response. */ +#[derive(JsonSchema, Serialize)] +#[doc(hidden)] +pub struct RedirectHeaders { + /** + * What type should we use to represent header values? + * + * It's tempting to use `http::HeaderValue` here. But in HTTP, header + * values can contain bytes that aren't valid Rust strings. See + * `http::header::HeaderValue`. We could propagate this nonsense all the + * way to the OpenAPI spec, encoding the Location header as, say, + * base64-encoded bytes. This sounds really annoying to consumers. It's + * also a fair bit more work to implement. We'd need to create a separate + * type for this field so that we can impl `Serialize` and `JsonSchema` on + * it, and we'd need to also impl serialization of byte sequences in + * `MapSerializer`. Ugh. + * + * We could just use `String`. This might contain values that aren't valid + * in HTTP response headers! But we can at least validate that at runtime, + * and it sure is easier to implement! + * + * + * + * We use `http::HeaderValue` here to reflect that the value has been + * validated as a header. This is annoying for us (because + * `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for + * callers (because they have to do the conversion and handle the error + * case). But it's precise and puts this error handling in the place it + * should go. + */ + #[serde(serialize_with = "serialize_http_header_value")] + location: SerializedHttpHeaderValue, +} + +struct SerializedHttpHeaderValue(http::HeaderValue); + +fn serialize_http_header_value( + value: &SerializedHttpHeaderValue, + serializer: S, +) -> Result { + // Technically, a header value can contain any bytes. + let str_value = value.0.to_str().map_err(|error| { + serde::ser::Error::custom(format!( + "response header contained invalid characters: {:#}", + error + )) + })?; + serializer.serialize_str(str_value) +} + +impl JsonSchema for SerializedHttpHeaderValue { + fn schema_name() -> String { + ::schema_name() + } + + fn json_schema( + gen: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + ::json_schema(gen) + } +} + +/** See `http_response_found()` */ +pub type HttpResponseFound = + HttpResponseHeaders; + /** - * `HttpResponseFoundNoContent` represents an HTTP 302 "Found" response with no - * response body. + * `http_response_found` returns an HTTP 302 "Found" response with no response + * body. * - * The sole item, a `String`, will become the value of the `Location` header. - * This is where you want to redirect the client to. + * The sole argument will become the value of the `Location` header. This is + * where you want to redirect the client to. * * Per MDN and RFC 9110 S15.4.3, you might want to use 307 ("Temporary * Redirect") or 303 ("See Other") instead. */ -pub struct HttpResponseFoundNoContent(pub String); +pub fn http_response_found(location: http::HeaderValue) -> HttpResponseFound { + HttpResponseHeaders::new( + HttpResponseFoundStatus, + RedirectHeaders { location: SerializedHttpHeaderValue(location) }, + ) +} -impl HttpCodedResponse for HttpResponseFoundNoContent { +/** + * This internal type impls HttpCodedResponse. External clients should use + * `HttpResponseFound` instead, which includes metadata about the `Location` + * header. + */ +#[doc(hidden)] +pub struct HttpResponseFoundStatus; +impl HttpCodedResponse for HttpResponseFoundStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::FOUND; const DESCRIPTION: &'static str = "redirect (302)"; } - -impl From for HttpHandlerResult { - fn from(response: HttpResponseFoundNoContent) -> HttpHandlerResult { - const STATUS_CODE: http::StatusCode = - ::STATUS_CODE; - Empty.to_response( - Response::builder() - .status(STATUS_CODE) - .header(http::header::LOCATION, &response.0), - ) +impl From for HttpHandlerResult { + fn from(_: HttpResponseFoundStatus) -> HttpHandlerResult { + HttpResponseFoundStatus::for_object(Empty) } } +/** See `http_response_see_other()` */ +pub type HttpResponseSeeOther = + HttpResponseHeaders; + /** - * `HttpResponseSeeOtherNoContent` represents an HTTP 303 "See Other" response - * with no response body. + * `http_response_see_other` returns an HTTP 303 "See Other" response with no + * response body. * - * The sole item, a `String`, will become the value of the `Location` header. - * This is where you want to redirect the client to. + * The sole argument will become the value of the `Location` header. This is + * where you want to redirect the client to. * * Use this (as opposed to 307 "Temporary Redirect") when you want the client to * follow up with a GET, rather than whatever method they used to make the * current request. This is intended to be used after a PUT or POST to show a * confirmation page or the like. */ -pub struct HttpResponseSeeOtherNoContent(pub String); +pub fn http_response_see_other( + location: http::HeaderValue, +) -> HttpResponseSeeOther { + HttpResponseHeaders::new( + HttpResponseSeeOtherStatus, + RedirectHeaders { location: SerializedHttpHeaderValue(location) }, + ) +} -impl HttpCodedResponse for HttpResponseSeeOtherNoContent { +/** + * This internal type impls HttpCodedResponse. External clients should use + * `HttpResponseSeeOther` instead, which includes metadata about the `Location` + * header. + */ +#[doc(hidden)] +pub struct HttpResponseSeeOtherStatus; +impl HttpCodedResponse for HttpResponseSeeOtherStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::SEE_OTHER; const DESCRIPTION: &'static str = "redirect (303)"; } - -impl From for HttpHandlerResult { - fn from(response: HttpResponseSeeOtherNoContent) -> HttpHandlerResult { - const STATUS_CODE: http::StatusCode = - ::STATUS_CODE; - Empty.to_response( - Response::builder() - .status(STATUS_CODE) - .header(http::header::LOCATION, &response.0), - ) +impl From for HttpHandlerResult { + fn from(_: HttpResponseSeeOtherStatus) -> HttpHandlerResult { + HttpResponseSeeOtherStatus::for_object(Empty) } } +/** See `http_response_temporary_redirect()` */ +pub type HttpResponseTemporaryRedirect = + HttpResponseHeaders; + /** - * `HttpResponseTemporaryRedirectNoContent` represents an HTTP 307 "Temporary + * `http_response_temporary_redirect` represents an HTTP 307 "Temporary * Redirect" response with no response body. * - * The sole item, a `String`, will become the value of the `Location` header. - * This is where you want to redirect the client to. + * The sole argument will become the value of the `Location` header. This is + * where you want to redirect the client to. * * Use this (as opposed to 303 "See Other") when you want the client to use the * same request method and body when it makes the follow-up request. */ -pub struct HttpResponseTemporaryRedirectNoContent(pub String); +pub fn http_response_temporary_redirect( + location: http::HeaderValue, +) -> HttpResponseTemporaryRedirect { + HttpResponseHeaders::new( + HttpResponseTemporaryRedirectStatus, + RedirectHeaders { location: SerializedHttpHeaderValue(location) }, + ) +} -impl HttpCodedResponse for HttpResponseTemporaryRedirectNoContent { +/** + * This internal type impls HttpCodedResponse. External clients should use + * `HttpResponseTemporaryRedirect` instead, which includes metadata about the + * `Location` header. + */ +#[doc(hidden)] +pub struct HttpResponseTemporaryRedirectStatus; +impl HttpCodedResponse for HttpResponseTemporaryRedirectStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::TEMPORARY_REDIRECT; const DESCRIPTION: &'static str = "redirect (307)"; } - -impl From for HttpHandlerResult { - fn from( - response: HttpResponseTemporaryRedirectNoContent, - ) -> HttpHandlerResult { - const STATUS_CODE: http::StatusCode = - ::STATUS_CODE; - Empty.to_response( - Response::builder() - .status(STATUS_CODE) - .header(http::header::LOCATION, &response.0), - ) +impl From for HttpHandlerResult { + fn from(_: HttpResponseTemporaryRedirectStatus) -> HttpHandlerResult { + HttpResponseTemporaryRedirectStatus::for_object(Empty) } } diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index 3d672bc64..12fd1fb62 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -634,6 +634,9 @@ pub use config::ConfigDropshot; pub use config::ConfigTls; pub use error::HttpError; pub use error::HttpErrorResponseBody; +pub use handler::http_response_found; +pub use handler::http_response_see_other; +pub use handler::http_response_temporary_redirect; pub use handler::Extractor; pub use handler::ExtractorMetadata; pub use handler::FreeformBody; @@ -642,11 +645,11 @@ pub use handler::HttpResponse; pub use handler::HttpResponseAccepted; pub use handler::HttpResponseCreated; pub use handler::HttpResponseDeleted; -pub use handler::HttpResponseFoundNoContent; +pub use handler::HttpResponseFound; pub use handler::HttpResponseHeaders; pub use handler::HttpResponseOk; -pub use handler::HttpResponseSeeOtherNoContent; -pub use handler::HttpResponseTemporaryRedirectNoContent; +pub use handler::HttpResponseSeeOther; +pub use handler::HttpResponseTemporaryRedirect; pub use handler::HttpResponseUpdatedNoContent; pub use handler::NoHeaders; pub use handler::Path; diff --git a/dropshot/tests/fail/bad_endpoint12.stderr b/dropshot/tests/fail/bad_endpoint12.stderr index f19d27868..664407b91 100644 --- a/dropshot/tests/fail/bad_endpoint12.stderr +++ b/dropshot/tests/fail/bad_endpoint12.stderr @@ -8,11 +8,11 @@ error[E0277]: the trait bound `String: HttpCodedResponse` is not satisfied HttpResponseAccepted HttpResponseCreated HttpResponseDeleted - HttpResponseFoundNoContent HttpResponseOk - HttpResponseSeeOtherNoContent - HttpResponseTemporaryRedirectNoContent HttpResponseUpdatedNoContent + dropshot::handler::HttpResponseFoundStatus + dropshot::handler::HttpResponseSeeOtherStatus + dropshot::handler::HttpResponseTemporaryRedirectStatus = note: required because of the requirements on the impl of `HttpResponse` for `String` note: required because of the requirements on the impl of `ResultTrait` for `Result` --> tests/fail/bad_endpoint12.rs:16:6 diff --git a/dropshot/tests/fail/bad_endpoint4.stderr b/dropshot/tests/fail/bad_endpoint4.stderr index 7476539c0..a45df7829 100644 --- a/dropshot/tests/fail/bad_endpoint4.stderr +++ b/dropshot/tests/fail/bad_endpoint4.stderr @@ -13,7 +13,7 @@ error[E0277]: the trait bound `QueryParams: schemars::JsonSchema` is not satisfi (T0, T1, T2, T3) (T0, T1, T2, T3, T4) (T0, T1, T2, T3, T4, T5) - and 144 others + and 145 others note: required by a bound in `dropshot::Query` --> src/handler.rs | diff --git a/dropshot/tests/fail/bad_endpoint7.stderr b/dropshot/tests/fail/bad_endpoint7.stderr index eb4e973a7..fe36e3fa6 100644 --- a/dropshot/tests/fail/bad_endpoint7.stderr +++ b/dropshot/tests/fail/bad_endpoint7.stderr @@ -13,7 +13,7 @@ error[E0277]: the trait bound `Ret: serde::ser::Serialize` is not satisfied (T0, T1, T2, T3) (T0, T1, T2, T3, T4) (T0, T1, T2, T3, T4, T5) - and 219 others + and 220 others = note: required because of the requirements on the impl of `dropshot::handler::HttpResponseContent` for `Ret` note: required by a bound in `HttpResponseOk` --> src/handler.rs diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index c14ba6aa3..a3f547e7b 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -17,6 +17,9 @@ use dropshot::channel; use dropshot::endpoint; +use dropshot::http_response_found; +use dropshot::http_response_see_other; +use dropshot::http_response_temporary_redirect; use dropshot::test_util::object_delete; use dropshot::test_util::read_json; use dropshot::test_util::read_string; @@ -25,11 +28,11 @@ use dropshot::test_util::TEST_HEADER_2; use dropshot::ApiDescription; use dropshot::HttpError; use dropshot::HttpResponseDeleted; -use dropshot::HttpResponseFoundNoContent; +use dropshot::HttpResponseFound; use dropshot::HttpResponseHeaders; use dropshot::HttpResponseOk; -use dropshot::HttpResponseSeeOtherNoContent; -use dropshot::HttpResponseTemporaryRedirectNoContent; +use dropshot::HttpResponseSeeOther; +use dropshot::HttpResponseTemporaryRedirect; use dropshot::HttpResponseUpdatedNoContent; use dropshot::Path; use dropshot::Query; @@ -1054,8 +1057,8 @@ async fn demo_handler_headers( }] async fn demo_handler_302_found( _rqctx: RequestCtx, -) -> Result { - Ok(HttpResponseFoundNoContent(String::from("/path1"))) +) -> Result { + Ok(http_response_found(http::HeaderValue::from_static("/path1"))) } #[endpoint { @@ -1064,8 +1067,8 @@ async fn demo_handler_302_found( }] async fn demo_handler_303_see_other( _rqctx: RequestCtx, -) -> Result { - Ok(HttpResponseSeeOtherNoContent(String::from("/path2"))) +) -> Result { + Ok(http_response_see_other(http::HeaderValue::from_static("/path2"))) } #[endpoint { @@ -1074,8 +1077,10 @@ async fn demo_handler_303_see_other( }] async fn demo_handler_307_temporary_redirect( _rqctx: RequestCtx, -) -> Result { - Ok(HttpResponseTemporaryRedirectNoContent(String::from("/path3"))) +) -> Result { + Ok(http_response_temporary_redirect(http::HeaderValue::from_static( + "/path3", + ))) } #[channel { diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 80f50e2b2..30721263f 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -293,7 +293,17 @@ "operationId": "handler21", "responses": { "302": { - "description": "redirect (302)" + "description": "redirect (302)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -312,7 +322,17 @@ "operationId": "handler22", "responses": { "303": { - "description": "redirect (303)" + "description": "redirect (303)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -331,7 +351,17 @@ "operationId": "handler23", "responses": { "307": { - "description": "redirect (307)" + "description": "redirect (307)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -940,6 +970,9 @@ "items" ] }, + "String": { + "type": "string" + }, "Foo": { "type": "string" } diff --git a/dropshot/tests/test_openapi.rs b/dropshot/tests/test_openapi.rs index defa331ca..c63024513 100644 --- a/dropshot/tests/test_openapi.rs +++ b/dropshot/tests/test_openapi.rs @@ -1,12 +1,13 @@ // Copyright 2022 Oxide Computer Company use dropshot::{ - endpoint, ApiDescription, FreeformBody, HttpError, HttpResponseAccepted, - HttpResponseCreated, HttpResponseDeleted, HttpResponseFoundNoContent, - HttpResponseHeaders, HttpResponseOk, HttpResponseSeeOtherNoContent, - HttpResponseTemporaryRedirectNoContent, HttpResponseUpdatedNoContent, - PaginationParams, Path, Query, RequestContext, ResultsPage, TagConfig, - TagDetails, TypedBody, UntypedBody, + endpoint, http_response_found, http_response_see_other, + http_response_temporary_redirect, ApiDescription, FreeformBody, HttpError, + HttpResponseAccepted, HttpResponseCreated, HttpResponseDeleted, + HttpResponseFound, HttpResponseHeaders, HttpResponseOk, + HttpResponseSeeOther, HttpResponseTemporaryRedirect, + HttpResponseUpdatedNoContent, PaginationParams, Path, Query, + RequestContext, ResultsPage, TagConfig, TagDetails, TypedBody, UntypedBody, }; use hyper::Body; use schemars::JsonSchema; @@ -428,8 +429,8 @@ async fn handler20( }] async fn handler21( _rqctx: Arc>, -) -> Result { - Ok(HttpResponseFoundNoContent(String::from("/path1"))) +) -> Result { + Ok(http_response_found(http::HeaderValue::from_static("/path1"))) } #[endpoint { @@ -439,8 +440,8 @@ async fn handler21( }] async fn handler22( _rqctx: Arc>, -) -> Result { - Ok(HttpResponseSeeOtherNoContent(String::from("/path2"))) +) -> Result { + Ok(http_response_see_other(http::HeaderValue::from_static("/path2"))) } #[endpoint { @@ -450,8 +451,10 @@ async fn handler22( }] async fn handler23( _rqctx: Arc>, -) -> Result { - Ok(HttpResponseTemporaryRedirectNoContent(String::from("/path3"))) +) -> Result { + Ok(http_response_temporary_redirect(http::HeaderValue::from_static( + "/path3", + ))) } fn make_api( diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index bae91e03a..6b741707e 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -301,7 +301,17 @@ "operationId": "handler21", "responses": { "302": { - "description": "redirect (302)" + "description": "redirect (302)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -320,7 +330,17 @@ "operationId": "handler22", "responses": { "303": { - "description": "redirect (303)" + "description": "redirect (303)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -339,7 +359,17 @@ "operationId": "handler23", "responses": { "307": { - "description": "redirect (307)" + "description": "redirect (307)", + "headers": { + "location": { + "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "style": "simple", + "required": true, + "schema": { + "$ref": "#/components/schemas/String" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -948,6 +978,9 @@ "items" ] }, + "String": { + "type": "string" + }, "Foo": { "type": "string" } From 3794efb3a136397f6e4ff06330592f26d8f83523 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 17:25:12 -0700 Subject: [PATCH 5/9] another attempt at incorporating headers --- dropshot/src/handler.rs | 79 +++++++------------------ dropshot/tests/test_demo.rs | 8 +-- dropshot/tests/test_openapi.json | 15 ++--- dropshot/tests/test_openapi.rs | 8 +-- dropshot/tests/test_openapi_fuller.json | 15 ++--- 5 files changed, 41 insertions(+), 84 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index fa268345b..2c5836b7e 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1453,49 +1453,11 @@ pub struct RedirectHeaders { * it, and we'd need to also impl serialization of byte sequences in * `MapSerializer`. Ugh. * - * We could just use `String`. This might contain values that aren't valid - * in HTTP response headers! But we can at least validate that at runtime, - * and it sure is easier to implement! - * - * - * - * We use `http::HeaderValue` here to reflect that the value has been - * validated as a header. This is annoying for us (because - * `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for - * callers (because they have to do the conversion and handle the error - * case). But it's precise and puts this error handling in the place it - * should go. + * We just use `String`. This might contain values that aren't valid in + * HTTP response headers. But we can at least validate that at runtime, and + * it sure is easier to implement! */ - #[serde(serialize_with = "serialize_http_header_value")] - location: SerializedHttpHeaderValue, -} - -struct SerializedHttpHeaderValue(http::HeaderValue); - -fn serialize_http_header_value( - value: &SerializedHttpHeaderValue, - serializer: S, -) -> Result { - // Technically, a header value can contain any bytes. - let str_value = value.0.to_str().map_err(|error| { - serde::ser::Error::custom(format!( - "response header contained invalid characters: {:#}", - error - )) - })?; - serializer.serialize_str(str_value) -} - -impl JsonSchema for SerializedHttpHeaderValue { - fn schema_name() -> String { - ::schema_name() - } - - fn json_schema( - gen: &mut schemars::gen::SchemaGenerator, - ) -> schemars::schema::Schema { - ::json_schema(gen) - } + location: String, } /** See `http_response_found()` */ @@ -1512,11 +1474,14 @@ pub type HttpResponseFound = * Per MDN and RFC 9110 S15.4.3, you might want to use 307 ("Temporary * Redirect") or 303 ("See Other") instead. */ -pub fn http_response_found(location: http::HeaderValue) -> HttpResponseFound { - HttpResponseHeaders::new( +pub fn http_response_found( + location: String, +) -> Result { + let _ = http::HeaderValue::from_str(&location)?; + Ok(HttpResponseHeaders::new( HttpResponseFoundStatus, - RedirectHeaders { location: SerializedHttpHeaderValue(location) }, - ) + RedirectHeaders { location }, + )) } /** @@ -1554,12 +1519,13 @@ pub type HttpResponseSeeOther = * confirmation page or the like. */ pub fn http_response_see_other( - location: http::HeaderValue, -) -> HttpResponseSeeOther { - HttpResponseHeaders::new( + location: String, +) -> Result { + let _ = http::HeaderValue::from_str(&location)?; + Ok(HttpResponseHeaders::new( HttpResponseSeeOtherStatus, - RedirectHeaders { location: SerializedHttpHeaderValue(location) }, - ) + RedirectHeaders { location }, + )) } /** @@ -1595,12 +1561,13 @@ pub type HttpResponseTemporaryRedirect = * same request method and body when it makes the follow-up request. */ pub fn http_response_temporary_redirect( - location: http::HeaderValue, -) -> HttpResponseTemporaryRedirect { - HttpResponseHeaders::new( + location: String, +) -> Result { + let _ = http::HeaderValue::from_str(&location)?; + Ok(HttpResponseHeaders::new( HttpResponseTemporaryRedirectStatus, - RedirectHeaders { location: SerializedHttpHeaderValue(location) }, - ) + RedirectHeaders { location }, + )) } /** diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index a3f547e7b..7083c2fcd 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -1058,7 +1058,7 @@ async fn demo_handler_headers( async fn demo_handler_302_found( _rqctx: RequestCtx, ) -> Result { - Ok(http_response_found(http::HeaderValue::from_static("/path1"))) + Ok(http_response_found(String::from("/path1")).unwrap()) } #[endpoint { @@ -1068,7 +1068,7 @@ async fn demo_handler_302_found( async fn demo_handler_303_see_other( _rqctx: RequestCtx, ) -> Result { - Ok(http_response_see_other(http::HeaderValue::from_static("/path2"))) + Ok(http_response_see_other(String::from("/path2")).unwrap()) } #[endpoint { @@ -1078,9 +1078,7 @@ async fn demo_handler_303_see_other( async fn demo_handler_307_temporary_redirect( _rqctx: RequestCtx, ) -> Result { - Ok(http_response_temporary_redirect(http::HeaderValue::from_static( - "/path3", - ))) + Ok(http_response_temporary_redirect(String::from("/path3")).unwrap()) } #[channel { diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 30721263f..53f818156 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -296,11 +296,11 @@ "description": "redirect (302)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -325,11 +325,11 @@ "description": "redirect (303)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -354,11 +354,11 @@ "description": "redirect (307)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -970,9 +970,6 @@ "items" ] }, - "String": { - "type": "string" - }, "Foo": { "type": "string" } diff --git a/dropshot/tests/test_openapi.rs b/dropshot/tests/test_openapi.rs index c63024513..3f7ce94e9 100644 --- a/dropshot/tests/test_openapi.rs +++ b/dropshot/tests/test_openapi.rs @@ -430,7 +430,7 @@ async fn handler20( async fn handler21( _rqctx: Arc>, ) -> Result { - Ok(http_response_found(http::HeaderValue::from_static("/path1"))) + Ok(http_response_found(String::from("/path1")).unwrap()) } #[endpoint { @@ -441,7 +441,7 @@ async fn handler21( async fn handler22( _rqctx: Arc>, ) -> Result { - Ok(http_response_see_other(http::HeaderValue::from_static("/path2"))) + Ok(http_response_see_other(String::from("/path2")).unwrap()) } #[endpoint { @@ -452,9 +452,7 @@ async fn handler22( async fn handler23( _rqctx: Arc>, ) -> Result { - Ok(http_response_temporary_redirect(http::HeaderValue::from_static( - "/path3", - ))) + Ok(http_response_temporary_redirect(String::from("/path3")).unwrap()) } fn make_api( diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 6b741707e..96cb55a47 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -304,11 +304,11 @@ "description": "redirect (302)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -333,11 +333,11 @@ "description": "redirect (303)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -362,11 +362,11 @@ "description": "redirect (307)", "headers": { "location": { - "description": "We use `http::HeaderValue` here to reflect that the value has been validated as a header. This is annoying for us (because `http::HeaderValue` doesn't implement JsonSchema or Serialize) and for callers (because they have to do the conversion and handle the error case). But it's precise and puts this error handling in the place it should go.", + "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", "style": "simple", "required": true, "schema": { - "$ref": "#/components/schemas/String" + "type": "string" } } } @@ -978,9 +978,6 @@ "items" ] }, - "String": { - "type": "string" - }, "Foo": { "type": "string" } From 9ab5165598db1514107c2d998a25fe0d92e429e1 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 17:29:28 -0700 Subject: [PATCH 6/9] fix doc comment --- dropshot/src/handler.rs | 3 ++- dropshot/tests/test_openapi.json | 6 +++--- dropshot/tests/test_openapi_fuller.json | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 2c5836b7e..35ae2ff0e 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1440,7 +1440,8 @@ impl From for HttpHandlerResult { #[derive(JsonSchema, Serialize)] #[doc(hidden)] pub struct RedirectHeaders { - /** + /** HTTP "Location" header */ + /* * What type should we use to represent header values? * * It's tempting to use `http::HeaderValue` here. But in HTTP, header diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 53f818156..6103524a7 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -296,7 +296,7 @@ "description": "redirect (302)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { @@ -325,7 +325,7 @@ "description": "redirect (303)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { @@ -354,7 +354,7 @@ "description": "redirect (307)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 96cb55a47..ab9ab7280 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -304,7 +304,7 @@ "description": "redirect (302)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { @@ -333,7 +333,7 @@ "description": "redirect (303)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { @@ -362,7 +362,7 @@ "description": "redirect (307)", "headers": { "location": { - "description": "What type should we use to represent header values?\n\nIt's tempting to use `http::HeaderValue` here. But in HTTP, header values can contain bytes that aren't valid Rust strings. See `http::header::HeaderValue`. We could propagate this nonsense all the way to the OpenAPI spec, encoding the Location header as, say, base64-encoded bytes. This sounds really annoying to consumers. It's also a fair bit more work to implement. We'd need to create a separate type for this field so that we can impl `Serialize` and `JsonSchema` on it, and we'd need to also impl serialization of byte sequences in `MapSerializer`. Ugh.\n\nWe just use `String`. This might contain values that aren't valid in HTTP response headers. But we can at least validate that at runtime, and it sure is easier to implement!", + "description": "HTTP \"Location\" header", "style": "simple", "required": true, "schema": { From 48655880ea1accb6e72abe4483da4a81caa87eb9 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 19:19:40 -0700 Subject: [PATCH 7/9] test empty bodies --- dropshot/tests/test_demo.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index 7083c2fcd..891882115 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -759,7 +759,7 @@ async fn test_header_request() { async fn test_302_found() { let api = demo_api(); let testctx = common::test_setup("test_302_found", api); - let response = testctx + let mut response = testctx .client_testctx .make_request( Method::GET, @@ -776,6 +776,7 @@ async fn test_302_found() { .map(|v| v.to_str().unwrap()) .collect::>(); assert_eq!(headers, vec!["/path1"]); + assert_eq!(read_string(&mut response).await, ""); } /* @@ -785,7 +786,7 @@ async fn test_302_found() { async fn test_303_see_other() { let api = demo_api(); let testctx = common::test_setup("test_303_see_other", api); - let response = testctx + let mut response = testctx .client_testctx .make_request( Method::GET, @@ -802,6 +803,7 @@ async fn test_303_see_other() { .map(|v| v.to_str().unwrap()) .collect::>(); assert_eq!(headers, vec!["/path2"]); + assert_eq!(read_string(&mut response).await, ""); } /* @@ -811,7 +813,7 @@ async fn test_303_see_other() { async fn test_307_temporary_redirect() { let api = demo_api(); let testctx = common::test_setup("test_307_temporary_redirect", api); - let response = testctx + let mut response = testctx .client_testctx .make_request( Method::GET, @@ -828,6 +830,7 @@ async fn test_307_temporary_redirect() { .map(|v| v.to_str().unwrap()) .collect::>(); assert_eq!(headers, vec!["/path3"]); + assert_eq!(read_string(&mut response).await, ""); } /* From 9a45f74a118d07ca71b96a9e2dfb212554ac7e87 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 28 Sep 2022 20:02:55 -0700 Subject: [PATCH 8/9] common error handling could be in dropshot --- dropshot/src/handler.rs | 25 +++++++++++++++++++------ dropshot/tests/test_demo.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 35ae2ff0e..3767e7035 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1477,14 +1477,25 @@ pub type HttpResponseFound = */ pub fn http_response_found( location: String, -) -> Result { - let _ = http::HeaderValue::from_str(&location)?; +) -> Result { + let _ = http::HeaderValue::from_str(&location) + .map_err(|e| http_redirect_error(e, &location))?; Ok(HttpResponseHeaders::new( HttpResponseFoundStatus, RedirectHeaders { location }, )) } +fn http_redirect_error( + error: http::header::InvalidHeaderValue, + location: &str, +) -> HttpError { + HttpError::for_internal_error(format!( + "error encoding redirect URL {:?}: {:#}", + location, error + )) +} + /** * This internal type impls HttpCodedResponse. External clients should use * `HttpResponseFound` instead, which includes metadata about the `Location` @@ -1521,8 +1532,9 @@ pub type HttpResponseSeeOther = */ pub fn http_response_see_other( location: String, -) -> Result { - let _ = http::HeaderValue::from_str(&location)?; +) -> Result { + let _ = http::HeaderValue::from_str(&location) + .map_err(|e| http_redirect_error(e, &location))?; Ok(HttpResponseHeaders::new( HttpResponseSeeOtherStatus, RedirectHeaders { location }, @@ -1563,8 +1575,9 @@ pub type HttpResponseTemporaryRedirect = */ pub fn http_response_temporary_redirect( location: String, -) -> Result { - let _ = http::HeaderValue::from_str(&location)?; +) -> Result { + let _ = http::HeaderValue::from_str(&location) + .map_err(|e| http_redirect_error(e, &location))?; Ok(HttpResponseHeaders::new( HttpResponseTemporaryRedirectStatus, RedirectHeaders { location }, diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index 891882115..aa628a37e 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -74,6 +74,7 @@ fn demo_api() -> ApiDescription { api.register(demo_handler_untyped_body).unwrap(); api.register(demo_handler_delete).unwrap(); api.register(demo_handler_headers).unwrap(); + api.register(demo_handler_302_bogus).unwrap(); api.register(demo_handler_302_found).unwrap(); api.register(demo_handler_303_see_other).unwrap(); api.register(demo_handler_307_temporary_redirect).unwrap(); @@ -752,6 +753,24 @@ async fn test_header_request() { assert_eq!(headers, vec!["hi", "howdy"]); } +/* + * Test 302 "Found" response with an invalid header value + */ +#[tokio::test] +async fn test_302_bogus() { + let api = demo_api(); + let testctx = common::test_setup("test_302_bogus", api); + let error = testctx + .client_testctx + .make_request_error( + Method::GET, + "/testing/302_bogus", + StatusCode::INTERNAL_SERVER_ERROR, + ) + .await; + assert_eq!(error.message, "Internal Server Error"); +} + /* * Test 302 "Found" response */ @@ -1054,6 +1073,16 @@ async fn demo_handler_headers( Ok(response) } +#[endpoint { + method = GET, + path = "/testing/302_bogus", +}] +async fn demo_handler_302_bogus( + _rqctx: RequestCtx, +) -> Result { + http_response_found(String::from("\x10")) +} + #[endpoint { method = GET, path = "/testing/302_found", From 74d5e18144135f632fbf9b6ac067985807f68014 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Thu, 29 Sep 2022 11:42:06 -0700 Subject: [PATCH 9/9] review edits --- dropshot/src/handler.rs | 14 +++++++------- dropshot/tests/test_openapi.json | 6 +++--- dropshot/tests/test_openapi_fuller.json | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 3767e7035..c05abc3ab 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -3,7 +3,7 @@ * Interface for implementing HTTP endpoint handler functions. * * For information about supported endpoint function signatures, argument types, - * extractors, and return types, see the top-level documentation dependencies: () for this crate. + * extractors, and return types, see the top-level documentation for this crate. * As documented there, we support several different sets of function arguments * and return types. * @@ -1497,7 +1497,7 @@ fn http_redirect_error( } /** - * This internal type impls HttpCodedResponse. External clients should use + * This internal type impls HttpCodedResponse. Consumers should use * `HttpResponseFound` instead, which includes metadata about the `Location` * header. */ @@ -1506,7 +1506,7 @@ pub struct HttpResponseFoundStatus; impl HttpCodedResponse for HttpResponseFoundStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::FOUND; - const DESCRIPTION: &'static str = "redirect (302)"; + const DESCRIPTION: &'static str = "redirect (found)"; } impl From for HttpHandlerResult { fn from(_: HttpResponseFoundStatus) -> HttpHandlerResult { @@ -1542,7 +1542,7 @@ pub fn http_response_see_other( } /** - * This internal type impls HttpCodedResponse. External clients should use + * This internal type impls HttpCodedResponse. Consumers should use * `HttpResponseSeeOther` instead, which includes metadata about the `Location` * header. */ @@ -1551,7 +1551,7 @@ pub struct HttpResponseSeeOtherStatus; impl HttpCodedResponse for HttpResponseSeeOtherStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::SEE_OTHER; - const DESCRIPTION: &'static str = "redirect (303)"; + const DESCRIPTION: &'static str = "redirect (see other)"; } impl From for HttpHandlerResult { fn from(_: HttpResponseSeeOtherStatus) -> HttpHandlerResult { @@ -1585,7 +1585,7 @@ pub fn http_response_temporary_redirect( } /** - * This internal type impls HttpCodedResponse. External clients should use + * This internal type impls HttpCodedResponse. Consumers should use * `HttpResponseTemporaryRedirect` instead, which includes metadata about the * `Location` header. */ @@ -1594,7 +1594,7 @@ pub struct HttpResponseTemporaryRedirectStatus; impl HttpCodedResponse for HttpResponseTemporaryRedirectStatus { type Body = Empty; const STATUS_CODE: StatusCode = StatusCode::TEMPORARY_REDIRECT; - const DESCRIPTION: &'static str = "redirect (307)"; + const DESCRIPTION: &'static str = "redirect (temporary redirect)"; } impl From for HttpHandlerResult { fn from(_: HttpResponseTemporaryRedirectStatus) -> HttpHandlerResult { diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 6103524a7..98e209d01 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -293,7 +293,7 @@ "operationId": "handler21", "responses": { "302": { - "description": "redirect (302)", + "description": "redirect (found)", "headers": { "location": { "description": "HTTP \"Location\" header", @@ -322,7 +322,7 @@ "operationId": "handler22", "responses": { "303": { - "description": "redirect (303)", + "description": "redirect (see other)", "headers": { "location": { "description": "HTTP \"Location\" header", @@ -351,7 +351,7 @@ "operationId": "handler23", "responses": { "307": { - "description": "redirect (307)", + "description": "redirect (temporary redirect)", "headers": { "location": { "description": "HTTP \"Location\" header", diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index ab9ab7280..b29b35fe9 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -301,7 +301,7 @@ "operationId": "handler21", "responses": { "302": { - "description": "redirect (302)", + "description": "redirect (found)", "headers": { "location": { "description": "HTTP \"Location\" header", @@ -330,7 +330,7 @@ "operationId": "handler22", "responses": { "303": { - "description": "redirect (303)", + "description": "redirect (see other)", "headers": { "location": { "description": "HTTP \"Location\" header", @@ -359,7 +359,7 @@ "operationId": "handler23", "responses": { "307": { - "description": "redirect (307)", + "description": "redirect (temporary redirect)", "headers": { "location": { "description": "HTTP \"Location\" header",