Skip to content

impl multi_value_headers #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Dec 26, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 85 additions & 8 deletions lambda-http/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{borrow::Cow, collections::HashMap, fmt, mem};

use http::{
self,
header::{HeaderValue, HOST},
header::{HeaderName, HeaderValue, HOST},
HeaderMap, Method, Request as HttpRequest,
};
use serde::{
Expand All @@ -31,6 +31,8 @@ pub(crate) struct GatewayRequest<'a> {
pub(crate) http_method: Method,
#[serde(deserialize_with = "deserialize_headers")]
pub(crate) headers: HeaderMap<HeaderValue>,
#[serde(default, deserialize_with = "deserialize_multi_value_headers")]
pub(crate) multi_value_headers: HeaderMap<HeaderValue>,
#[serde(deserialize_with = "nullable_default")]
pub(crate) query_string_parameters: StrMap,
#[serde(deserialize_with = "nullable_default")]
Expand Down Expand Up @@ -78,6 +80,7 @@ pub struct Identity {
pub user_arn: Option<String>,
}

/// Deserialize a str into an http::Method
fn deserialize_method<'de, D>(deserializer: D) -> Result<Method, D::Error>
where
D: Deserializer<'de>,
Expand All @@ -102,6 +105,49 @@ where
deserializer.deserialize_str(MethodVisitor)
}

/// Deserialize a map of Cow<'_, str> => Vec<Cow<'_, str>> into an http::HeaderMap
fn deserialize_multi_value_headers<'de, D>(deserializer: D) -> Result<HeaderMap<HeaderValue>, D::Error>
where
D: Deserializer<'de>,
{
struct HeaderVisitor;

impl<'de> Visitor<'de> for HeaderVisitor {
type Value = HeaderMap<HeaderValue>;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(formatter, "a multi valued HeaderMap<HeaderValue>")
}

fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
where
A: MapAccess<'de>,
{
let mut headers = map
.size_hint()
.map(HeaderMap::with_capacity)
.unwrap_or_else(HeaderMap::new);
while let Some((key, values)) = map.next_entry::<Cow<'_, str>, Vec<Cow<'_, str>>>()? {
// note the aws docs for multi value headers include an empty key. I'm not sure if this is a doc bug
// or not by the http crate doesn't handle it
// https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-input-format
if !key.is_empty() {
for value in values {
let header_name = key.parse::<HeaderName>().map_err(A::Error::custom)?;
let header_value =
HeaderValue::from_shared(value.into_owned().into()).map_err(A::Error::custom)?;
headers.append(header_name, header_value);
}
}
}
Ok(headers)
}
}

deserializer.deserialize_map(HeaderVisitor)
}

/// Deserialize a map of Cow<'_, str> => Cow<'_, str> into an http::HeaderMap
fn deserialize_headers<'de, D>(deserializer: D) -> Result<HeaderMap<HeaderValue>, D::Error>
where
D: Deserializer<'de>,
Expand All @@ -119,11 +165,13 @@ where
where
A: MapAccess<'de>,
{
let mut headers = http::HeaderMap::new();
let mut headers = map
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took a liberty to make a small optimization here. serde's internal map model may provide a size hint for those building up deserialized maps to pre-allocate an appropriately sized map. I'm doing that here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

.size_hint()
.map(HeaderMap::with_capacity)
.unwrap_or_else(HeaderMap::new);
while let Some((key, value)) = map.next_entry::<Cow<'_, str>, Cow<'_, str>>()? {
let header_name = key.parse::<http::header::HeaderName>().map_err(A::Error::custom)?;
let header_value =
http::header::HeaderValue::from_shared(value.into_owned().into()).map_err(A::Error::custom)?;
let header_name = key.parse::<HeaderName>().map_err(A::Error::custom)?;
let header_value = HeaderValue::from_shared(value.into_owned().into()).map_err(A::Error::custom)?;
headers.append(header_name, header_value);
}
Ok(headers)
Expand All @@ -150,6 +198,7 @@ impl<'a> From<GatewayRequest<'a>> for HttpRequest<Body> {
path,
http_method,
headers,
mut multi_value_headers,
query_string_parameters,
path_parameters,
stage_variables,
Expand Down Expand Up @@ -191,8 +240,20 @@ impl<'a> From<GatewayRequest<'a>> for HttpRequest<Body> {
})
.expect("failed to build request");

// merge headers into multi_value_headers and make
// multi_value_headers our cannoncial source of request headers
for (key, value) in headers {
// see HeaderMap#into_iter() docs for cases when key element may be None
if let Some(first_key) = key {
// if it contains the key, avoid appending a duplicate value
if !multi_value_headers.contains_key(&first_key) {
multi_value_headers.append(first_key, value);
}
}
}

// no builder method that sets headers in batch
mem::replace(req.headers_mut(), headers);
mem::replace(req.headers_mut(), multi_value_headers);

req
}
Expand Down Expand Up @@ -224,8 +285,24 @@ mod tests {
fn deserializes_request_events() {
// from the docs
// https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-api-gateway-request
let input = include_str!("../tests/data/proxy_request.json");
assert!(serde_json::from_str::<GatewayRequest<'_>>(&input).is_ok())
let input = include_str!("../tests/data/apigw_proxy_request.json");
let result = serde_json::from_str::<GatewayRequest<'_>>(&input);
assert!(
result.is_ok(),
format!("event is was not parsed as expected {:?}", result)
);
}

#[test]
fn deserialize_multi_value_events() {
// from docs
// https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-input-format
let input = include_str!("../tests/data/apigw_multi_value_proxy_request.json");
let result = serde_json::from_str::<GatewayRequest<'_>>(&input);
assert!(
result.is_ok(),
format!("event is was not parsed as expected {:?}", result)
);
}

#[test]
Expand Down
45 changes: 43 additions & 2 deletions lambda-http/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::ops::Not;

use http::{
header::{HeaderMap, HeaderValue},
header::{HeaderMap, HeaderValue, CONTENT_TYPE},
Response,
};
use serde::{
Expand All @@ -22,23 +22,47 @@ pub(crate) struct GatewayResponse {
pub status_code: u16,
#[serde(skip_serializing_if = "HeaderMap::is_empty", serialize_with = "serialize_headers")]
pub headers: HeaderMap<HeaderValue>,
#[serde(
skip_serializing_if = "HeaderMap::is_empty",
serialize_with = "serialize_multi_value_headers"
)]
pub multi_value_headers: HeaderMap<HeaderValue>,
#[serde(skip_serializing_if = "Option::is_none")]
pub body: Option<Body>,
#[serde(skip_serializing_if = "Not::not")]
pub is_base64_encoded: bool,
}

#[cfg(test)]
impl Default for GatewayResponse {
fn default() -> Self {
Self {
status_code: 200,
headers: Default::default(),
multi_value_headers: Default::default(),
body: Default::default(),
is_base64_encoded: Default::default(),
}
}
}

/// Serialize a http::HeaderMap into a serde str => str map
fn serialize_multi_value_headers<S>(headers: &HeaderMap<HeaderValue>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let mut map = serializer.serialize_map(Some(headers.keys_len()))?;
for key in headers.keys() {
let mut map_values = Vec::new();
for value in headers.get_all(key) {
map_values.push(value.to_str().map_err(S::Error::custom)?)
}
map.serialize_entry(key.as_str(), &map_values)?;
}
map.end()
}

/// Serialize a http::HeaderMap into a serde str => Vec<str> map
fn serialize_headers<S>(headers: &HeaderMap<HeaderValue>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
Expand All @@ -65,7 +89,8 @@ where
GatewayResponse {
status_code: parts.status.as_u16(),
body,
headers: parts.headers,
headers: parts.headers.clone(),
multi_value_headers: parts.headers,
is_base64_encoded,
}
}
Expand Down Expand Up @@ -127,6 +152,7 @@ impl IntoResponse for serde_json::Value {
#[cfg(test)]
mod tests {
use super::{Body, GatewayResponse, IntoResponse};
use http::Response;
use serde_json::{self, json};

#[test]
Expand Down Expand Up @@ -176,4 +202,19 @@ mod tests {
r#"{"statusCode":200,"body":"foo"}"#
);
}

#[test]
fn serialize_multi_value_headers() {
let res: GatewayResponse = Response::builder()
.header("multi", "a")
.header("multi", "b")
.body(Body::from(()))
.expect("failed to create request")
.into();
let json = serde_json::to_string(&res).expect("failed to serialize to json");
assert_eq!(
json,
r#"{"statusCode":200,"headers":{"multi":"a"},"multiValueHeaders":{"multi":["a","b"]}}"#
)
}
}
131 changes: 131 additions & 0 deletions lambda-http/tests/data/apigw_multi_value_proxy_request.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
{
"resource": "/{proxy+}",
"path": "/hello/world",
"httpMethod": "POST",
"headers": {
"Accept": "*/*",
"Accept-Encoding": "gzip, deflate",
"cache-control": "no-cache",
"CloudFront-Forwarded-Proto": "https",
"CloudFront-Is-Desktop-Viewer": "true",
"CloudFront-Is-Mobile-Viewer": "false",
"CloudFront-Is-SmartTV-Viewer": "false",
"CloudFront-Is-Tablet-Viewer": "false",
"CloudFront-Viewer-Country": "US",
"Content-Type": "application/json",
"headerName": "headerValue",
"Host": "gy415nuibc.execute-api.us-east-1.amazonaws.com",
"Postman-Token": "9f583ef0-ed83-4a38-aef3-eb9ce3f7a57f",
"User-Agent": "PostmanRuntime/2.4.5",
"Via": "1.1 d98420743a69852491bbdea73f7680bd.cloudfront.net (CloudFront)",
"X-Amz-Cf-Id": "pn-PWIJc6thYnZm5P0NMgOUglL1DYtl0gdeJky8tqsg8iS_sgsKD1A==",
"X-Forwarded-For": "54.240.196.186, 54.182.214.83",
"X-Forwarded-Port": "443",
"X-Forwarded-Proto": "https"
},
"multiValueHeaders":{
"Accept":[
"*/*"
],
"Accept-Encoding":[
"gzip, deflate"
],
"cache-control":[
"no-cache"
],
"CloudFront-Forwarded-Proto":[
"https"
],
"CloudFront-Is-Desktop-Viewer":[
"true"
],
"CloudFront-Is-Mobile-Viewer":[
"false"
],
"CloudFront-Is-SmartTV-Viewer":[
"false"
],
"CloudFront-Is-Tablet-Viewer":[
"false"
],
"CloudFront-Viewer-Country":[
"US"
],
"":[
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is a documentation bug but this is what aws has documented I'm not sure what the right external channels are but you may want to leverage internal channels to notify the team that maintains these or clarify in the docs what an empty by present header name means :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question; will do. Not entirely sure myself...

""
],
"Content-Type":[
"application/json"
],
"headerName":[
"headerValue"
],
"Host":[
"gy415nuibc.execute-api.us-east-1.amazonaws.com"
],
"Postman-Token":[
"9f583ef0-ed83-4a38-aef3-eb9ce3f7a57f"
],
"User-Agent":[
"PostmanRuntime/2.4.5"
],
"Via":[
"1.1 d98420743a69852491bbdea73f7680bd.cloudfront.net (CloudFront)"
],
"X-Amz-Cf-Id":[
"pn-PWIJc6thYnZm5P0NMgOUglL1DYtl0gdeJky8tqsg8iS_sgsKD1A=="
],
"X-Forwarded-For":[
"54.240.196.186, 54.182.214.83"
],
"X-Forwarded-Port":[
"443"
],
"X-Forwarded-Proto":[
"https"
]
},
"queryStringParameters": {
"name": "me",
"multivalueName": "me"
},
"multiValueQueryStringParameters":{
"name":[
"me"
],
"multivalueName":[
"you",
"me"
]
},
"pathParameters": {
"proxy": "hello/world"
},
"stageVariables": {
"stageVariableName": "stageVariableValue"
},
"requestContext": {
"accountId": "12345678912",
"resourceId": "roq9wj",
"stage": "testStage",
"requestId": "deef4878-7910-11e6-8f14-25afc3e9ae33",
"identity": {
"cognitoIdentityPoolId": null,
"accountId": null,
"cognitoIdentityId": null,
"caller": null,
"apiKey": null,
"sourceIp": "192.168.196.186",
"cognitoAuthenticationType": null,
"cognitoAuthenticationProvider": null,
"userArn": null,
"userAgent": "PostmanRuntime/2.4.5",
"user": null
},
"resourcePath": "/{proxy+}",
"httpMethod": "POST",
"apiId": "gy415nuibc"
},
"body": "{\r\n\t\"a\": 1\r\n}",
"isBase64Encoded": false
}