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
fix(adapter): handle multi value headers in AWS Lambda #2494
Conversation
Hi @exoego Thank you for the PR. Looks good to me. But I'd like @watany-dev to review it. @watany-dev Can you do it? |
@@ -27,3 +29,69 @@ describe('isContentEncodingBinary', () => { | |||
expect(isContentEncodingBinary('unknown')).toBe(false) | |||
}) | |||
}) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is complicated, but it seems better to maintain runtime_tests/lambda
instead of here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to d51f022
And added tests for APIGW V1 and V2 as well.
@@ -22,6 +22,7 @@ export interface APIGatewayProxyEventV2 { | |||
version: string | |||
routeKey: string | |||
headers: Record<string, string | undefined> | |||
multiValueHeaders?: undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is your rationale for this? The documentation does not appear to include multiValueHeaders in ver 2.0. https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It simplifies code in my opinion.
It allows
event.multiValueHeaders?.['host']?.[0]
instead of
'multiValueHeaders' in event
? event.multiValueHeaders.['host']?.[0]
: undefined
and it also allows
} else if (event.multiValueHeaders) {
for (const [k, values] of
Object.entries(event.multiValueHeaders)) {
instead of
} else if ('multiValueHeaders' in event) {
for (const [k, values] of
Object.entries(event.multiValueHeaders || {})) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. The implementation is clean, and I think this is good.
Note.
Below is a note for when the need arises to completely remove 'multiValueHeaders'.
const createRequest = (event: LambdaEvent) => {
const queryString = extractQueryString(event)
const domainName =
event.requestContext && 'domainName' in event.requestContext
? event.requestContext.domainName
: getHostFromEvent(event)
const path = isProxyEventV2(event) ? event.rawPath : event.path
const urlPath = `https://${domainName}${path}`
const url = queryString ? `${urlPath}?${queryString}` : urlPath
const headers = new Headers()
getCookies(event, headers)
if (event.headers) {
for (const [k, v] of Object.entries(event.headers)) {
if (v) {
headers.set(k, v)
}
}
}
if (isAPIGatewayProxyEvent(event) && event.multiValueHeaders) {
for (const [k, values] of Object.entries(event.multiValueHeaders)) {
if (values) {
values.forEach((v) => headers.append(k, v))
}
}
}
const method = isProxyEventV2(event) ? event.requestContext.http.method : event.httpMethod
const requestInit: RequestInit = {
headers,
method,
}
if (event.body) {
requestInit.body = event.isBase64Encoded ? Buffer.from(event.body, 'base64') : event.body
}
return new Request(url, requestInit)
}
const getHostFromEvent = (event: LambdaEvent): string => {
if (!event.headers) {
return 'localhost'
}
if (isAPIGatewayProxyEvent(event)) {
return event.headers['host'] ?? event.multiValueHeaders?.['host']?.[0] ?? 'localhost'
} else {
return event.headers['host'] ?? 'localhost'
}
}
const isAPIGatewayProxyEvent = (event: LambdaEvent): event is APIGatewayProxyEvent => {
return 'multiValueHeaders' in event
}
@@ -63,7 +64,8 @@ export interface APIGatewayProxyEvent { | |||
// When calling Lambda through an Application Load Balancer | |||
export interface ALBProxyEvent { | |||
httpMethod: string | |||
headers: Record<string, string | undefined> | |||
headers?: Record<string, string | undefined> | |||
multiValueHeaders?: Record<string, string[] | undefined> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@exoego |
@watany-dev Please take a look 🙇 |
Hey @watany-dev, Can you see this? |
} | ||
if (event.multiValueHeaders) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note) This is not } else if
because headers
are not optional, at least in type definition in this repo.
It means that we need to take care of headers
and multiValueHeaders
in tests.
@watany-dev Thanks! I also think it looks good. @exoego Can I merge this now? |
@yusukebe Yessss, it's ready to be merged |
Okay! Let's go! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [hono](https://hono.dev/) ([source](https://togithub.com/honojs/hono)) | [`4.1.4` -> `4.2.7`](https://renovatebot.com/diffs/npm/hono/4.1.4/4.2.7) | [![age](https://developer.mend.io/api/mc/badges/age/npm/hono/4.2.7?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/hono/4.2.7?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/hono/4.1.4/4.2.7?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/hono/4.1.4/4.2.7?slim=true)](https://docs.renovatebot.com/merge-confidence/) | ### GitHub Vulnerability Alerts #### [CVE-2024-32869](https://togithub.com/honojs/hono/security/advisories/GHSA-3mpf-rcc7-5347) ### Summary When using serveStatic with deno, it is possible to directory traverse where main.ts is located. My environment is configured as per this tutorial https://hono.dev/getting-started/deno ### PoC ```bash $ tree . ├── deno.json ├── deno.lock ├── main.ts ├── README.md └── static └── a.txt ``` source ```jsx import { Hono } from 'https://deno.land/x/hono@v4.2.6/mod.ts' import { serveStatic } from 'https://deno.land/x/hono@v4.2.6/middleware.ts' const app = new Hono() app.use('/static/*', serveStatic({ root: './' })) Deno.serve(app.fetch) ``` request ```bash curl localhost:8000/static/%2e%2e/main.ts ``` response is content of main.ts ### Impact Unexpected files are retrieved. --- ### Release Notes <details> <summary>honojs/hono (hono)</summary> ### [`v4.2.7`](https://togithub.com/honojs/hono/releases/tag/v4.2.7) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.6...v4.2.7) This release fixes "[Restricted Directory Traversal in serveStatic with deno](https://togithub.com/honojs/hono/security/advisories/GHSA-3mpf-rcc7-5347)". **Full Changelog**: honojs/hono@v4.2.6...v4.2.7 ### [`v4.2.6`](https://togithub.com/honojs/hono/releases/tag/v4.2.6) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.5...v4.2.6) #### What's Changed - refactor(adapter/aws): Optimize multiple call of same conditions with polymorphism by [@​exoego](https://togithub.com/exoego) in [honojs/hono#2521 - fix(sse): close sse stream on end by [@​domeccleston](https://togithub.com/domeccleston) in [honojs/hono#2529 - fix(client): Don't show `$ws` when not used WebSockets by [@​nakasyou](https://togithub.com/nakasyou) in [honojs/hono#2532 - refactor(ssg): update utils.ts by [@​eltociear](https://togithub.com/eltociear) in [honojs/hono#2519 #### New Contributors - [@​domeccleston](https://togithub.com/domeccleston) made their first contribution in [honojs/hono#2529 - [@​eltociear](https://togithub.com/eltociear) made their first contribution in [honojs/hono#2519 **Full Changelog**: honojs/hono@v4.2.5...v4.2.6 ### [`v4.2.5`](https://togithub.com/honojs/hono/releases/tag/v4.2.5) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.4...v4.2.5) #### What's Changed - fix(client): Allow calling toString and valueOf on the proxy object by [@​ibash](https://togithub.com/ibash) in [honojs/hono#2510 - fix(adapter): handle multi value headers in AWS Lambda by [@​exoego](https://togithub.com/exoego) in [honojs/hono#2494 - fix(client): shuold not remove tailing slash from top-level URL by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2523 - fix(jsx/dom): remove lookbehind assertion in event regexp by [@​usualoma](https://togithub.com/usualoma) in [honojs/hono#2524 #### New Contributors - [@​ibash](https://togithub.com/ibash) made their first contribution in [honojs/hono#2510 **Full Changelog**: honojs/hono@v4.2.4...v4.2.5 ### [`v4.2.4`](https://togithub.com/honojs/hono/releases/tag/v4.2.4) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.3...v4.2.4) ##### What's Changed - fix(jwt): Make JWT Header `typ` Field Optional to Enhance Compatibility by [@​naporin0624](https://togithub.com/naporin0624) in [honojs/hono#2488 - fix(testing): set `baseUrl` for `testClient` by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2496 - fix(validator): Default use to `OutputTypeExcludeResponseType` when `InputType` is unknown by [@​nagasawaryoya](https://togithub.com/nagasawaryoya) in [honojs/hono#2500 - refactor(trie-router): parentPatterns is updated but never queried by [@​exoego](https://togithub.com/exoego) in [honojs/hono#2503 - refactor: Remove redundant initializer by [@​exoego](https://togithub.com/exoego) in [honojs/hono#2502 - refactor(cloudflare-workers): Suppress eslint noise by [@​exoego](https://togithub.com/exoego) in [honojs/hono#2504 - fix(jsx): Add catch to async function's promise by [@​mwilkins91](https://togithub.com/mwilkins91) in [honojs/hono#2471 ##### New Contributors - [@​nagasawaryoya](https://togithub.com/nagasawaryoya) made their first contribution in [honojs/hono#2500 - [@​exoego](https://togithub.com/exoego) made their first contribution in [honojs/hono#2503 - [@​mwilkins91](https://togithub.com/mwilkins91) made their first contribution in [honojs/hono#2471 **Full Changelog**: honojs/hono@v4.2.3...v4.2.4 ### [`v4.2.3`](https://togithub.com/honojs/hono/releases/tag/v4.2.3) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.2...v4.2.3) #### What's Changed - fix(ssg): use response header to mark as disabled routes for SSG by [@​usualoma](https://togithub.com/usualoma) in [honojs/hono#2477 - fix(trailing-slash): export types in `package.json` correctly by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2483 - fix(client): fix websocket client protocol by [@​naporin0624](https://togithub.com/naporin0624) in [honojs/hono#2479 **Full Changelog**: honojs/hono@v4.2.2...v4.2.3 ### [`v4.2.2`](https://togithub.com/honojs/hono/releases/tag/v4.2.2) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.1...v4.2.2) #### What's Changed - feat(jsx-renderer): pass the context as 2nd arg by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2459 - feat(client): accept a function that provides dynamic headers to hc by [@​niko-gardenanet](https://togithub.com/niko-gardenanet) in [honojs/hono#2461 - fix(client): infer `null` correctly by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2469 #### New Contributors - [@​niko-gardenanet](https://togithub.com/niko-gardenanet) made their first contribution in [honojs/hono#2461 **Full Changelog**: honojs/hono@v4.2.1...v4.2.2 ### [`v4.2.1`](https://togithub.com/honojs/hono/releases/tag/v4.2.1) [Compare Source](https://togithub.com/honojs/hono/compare/v4.2.0...v4.2.1) #### What's Changed - fix(jws): Only import necessary helper (not all helpers) by [@​nicksrandall](https://togithub.com/nicksrandall) in [honojs/hono#2458 #### New Contributors - [@​nicksrandall](https://togithub.com/nicksrandall) made their first contribution in [honojs/hono#2458 **Full Changelog**: honojs/hono@v4.2.0...v4.2.1 ### [`v4.2.0`](https://togithub.com/honojs/hono/releases/tag/v4.2.0) [Compare Source](https://togithub.com/honojs/hono/compare/v4.1.7...v4.2.0) Hono v4.2.0 is now available! Let's take a look at the new features. #### Added more algorithms for JWT The number of algorithms that JWT util can handle has increased from only 3 to 13! This means that JWT util now implements many of the algorithms supported by JWT. - HS256 - HS384 - HS512 - RS256 - RS384 - RS512 - PS256 - PS384 - PS512 - ES256 - ES384 - ES512 - EdDSA You can use these algorithms from the JWT middleware or JWT helpers. Thanks [@​Code-Hex](https://togithub.com/Code-Hex)! #### Method Override Middleware [Method Override Middleware](https://hono.dev/middleware/builtin/method-override) has been added. This middleware override the method of the real request with the specified method. HTML `form` does not allow you to send a DELETE method request. Instead, by sending an input with `name` as `_method` and a value of `DELETE`, you can call the handler registered in `app.delete()`. ```ts const app = new Hono() // If no options are specified, the value of `_method` in the form, // e.g. DELETE, is used as the method. app.use('/posts', methodOverride({ app })) app.delete('/posts', (c) => { // .... }) ``` #### Trailing Slash Middleware [Trailing Slash Middleware](https://hono.dev/middleware/builtin/trailing-slash) resolves the handling of Trailing Slashes in GET requests. You can use `appendTrailingSlash` and `trimTrailingSlash` functions. For example, it redirects a GET request to `/about/me` to `/about/me/`. ```ts import { Hono } from 'hono' import { appendTrailingSlash } from 'hono/trailing-slash' const app = new Hono({ strict: true }) app.use(appendTrailingSlash()) app.get('/about/me/', (c) => c.text('With Trailing Slash')) ``` Thanks [@​rnmeow](https://togithub.com/rnmeow)! #### Other features - SSG Helper - Support `extensionMap` [honojs/hono#2382 - JSX/DOM - Add `userId` hook [honojs/hono#2389 - JWT Middleware - Improve error handling [honojs/hono#2406 - Request - Cache the body for re-using [honojs/hono#2416 - JWT Util - Add type helper to `payload` [honojs/hono#2424 - CORS Middleware - Pass context to `options.origin` function [honojs/hono#2436 - Cache Middleware - Support for the `vary` header option [honojs/hono#2426 - HTTP Exception - Add `cause` option [honojs/hono#2224 - Logger - Support `NO_COLOR` [honojs/hono#2228 - JWT Middleware - Add `JwtTokenInvalid` object as `cause` when JWT is invalid [honojs/hono#2448 - Bearer Auth Middleware - Add `verifyToken` option [honojs/hono#2449 - Basic Auth Middleware - Add `verifyUser` option [honojs/hono#2450 #### All Updates - feat(jwt): supported RS256, RS384, RS512 algorithm for JWT by [@​Code-Hex](https://togithub.com/Code-Hex) in [honojs/hono#2339 - added remain algorithm for JWT by [@​Code-Hex](https://togithub.com/Code-Hex) in [honojs/hono#2352 - acceptable CryptoKey in JWT sign and verify by [@​Code-Hex](https://togithub.com/Code-Hex) in [honojs/hono#2373 - feat(ssg): Support `extentionMap` by [@​watany-dev](https://togithub.com/watany-dev) in [honojs/hono#2382 - feat(jwt): support remaining algorithms by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2368 - feat(jsx): add useId hook by [@​usualoma](https://togithub.com/usualoma) in [honojs/hono#2389 - feat(middleware/jwt): improve error handling by [@​tfkhdyt](https://togithub.com/tfkhdyt) in [honojs/hono#2406 - feat(request): cache body for reusing by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2416 - feat(jwt): Add type helper to `payload` by [@​nakasyou](https://togithub.com/nakasyou) in [honojs/hono#2424 - feat: introduce Method Override Middleware by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2420 - feat(middleware/cors): pass context to options.origin function by [@​okmr-d](https://togithub.com/okmr-d) in [honojs/hono#2436 - feat: support for `vary` header in cache middleware by [@​naporin0624](https://togithub.com/naporin0624) in [honojs/hono#2426 - feat: add middlewares resolve trailing slashes on GET request by [@​rnmeow](https://togithub.com/rnmeow) in [honojs/hono#2408 - test: stub `crypto` if not exist by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2445 - feat(jwt): literal typed `alg` option value by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2446 - test(ssg): add test for content-type includes `;` by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2447 - feat(jwt): add `JwtTokenInvalid` object as `cause` when JWT is invalid by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2448 - feat(bearer-auth): add `verifyToken` option by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2449 - feat(basic-auth): add `verifyUser` option by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2450 - Next by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2454 #### New Contributors - [@​tfkhdyt](https://togithub.com/tfkhdyt) made their first contribution in [honojs/hono#2406 - [@​okmr-d](https://togithub.com/okmr-d) made their first contribution in [honojs/hono#2436 - [@​naporin0624](https://togithub.com/naporin0624) made their first contribution in [honojs/hono#2426 - [@​rnmeow](https://togithub.com/rnmeow) made their first contribution in [honojs/hono#2408 **Full Changelog**: honojs/hono@v4.1.7...v4.2.0 ### [`v4.1.7`](https://togithub.com/honojs/hono/releases/tag/v4.1.7) [Compare Source](https://togithub.com/honojs/hono/compare/v4.1.6...v4.1.7) #### What's Changed - fix(cache): check `globalThis.caches` by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2444 **Full Changelog**: honojs/hono@v4.1.6...v4.1.7 ### [`v4.1.6`](https://togithub.com/honojs/hono/releases/tag/v4.1.6) [Compare Source](https://togithub.com/honojs/hono/compare/v4.1.5...v4.1.6) #### What's Changed - chore(benchmark): add "loop" script by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2431 - fix(cache): not enabled if `caches` is not defined by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2443 **Full Changelog**: honojs/hono@v4.1.5...v4.1.6 ### [`v4.1.5`](https://togithub.com/honojs/hono/releases/tag/v4.1.5) [Compare Source](https://togithub.com/honojs/hono/compare/v4.1.4...v4.1.5) #### What's Changed - perf: Don't use `Arrap.prototype.map` if it is not needed return value by [@​nakasyou](https://togithub.com/nakasyou) in [honojs/hono#2419 - fix(aws-lambda): handle response without body ([#​2401](https://togithub.com/honojs/hono/issues/2401)) by [@​KnisterPeter](https://togithub.com/KnisterPeter) in [honojs/hono#2413 - fix(validator): `await` cached contents by [@​yusukebe](https://togithub.com/yusukebe) in [honojs/hono#2430 #### New Contributors - [@​KnisterPeter](https://togithub.com/KnisterPeter) made their first contribution in [honojs/hono#2413 **Full Changelog**: honojs/hono@v4.1.4...v4.1.5 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "" in timezone America/Chicago, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/autoblocksai/cli). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMTMuMSIsInVwZGF0ZWRJblZlciI6IjM3LjMxMy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Fixes #2493
Fixes #2495
Fixes for #2493
The current implementation assumes
headers
are always defined, which is incorrectheaders
can beundefined
ifmultiValueHeaders
exists,, and vice-versa, sinceheaders
andmultiValueHeaders
are mutually exclusive.aws-lambda
type definition in DefinitelyTyped also does so:Fixes for #2495
This PR also fixes a conversion from Lambda Event to Request for all event types (ALB, API GW V1/V2).
The current implementation does not extract values from multi-value headers.
The new impl. extract values and delegates those to Request objects, using Header#append.
Author should do the followings, if applicable
yarn denoify
to generate files for Deno