Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
5 changes: 5 additions & 0 deletions .changeset/seven-ducks-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/aws": patch
---

fix: Avoid merging Location header on response when its an array
23 changes: 22 additions & 1 deletion packages/open-next/src/http/util.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type http from "node:http";
import logger from "../logger";

export const parseHeaders = (
headers?: http.OutgoingHttpHeader[] | http.OutgoingHttpHeaders,
Expand All @@ -12,7 +13,27 @@ export const parseHeaders = (
if (value === undefined) {
continue;
}
result[key.toLowerCase()] = convertHeader(value);
const keyLower = key.toLowerCase();
/**
* Next can return an Array for the Location header
* We dont want to merge that into a comma-separated string
* If they are the same just return one of them
* Otherwise return the last one
* See: https://github.com/opennextjs/opennextjs-cloudflare/issues/875#issuecomment-3258248276
* and https://github.com/opennextjs/opennextjs-aws/pull/977#issuecomment-3261763114
*/
if (keyLower === "location" && Array.isArray(value)) {
if (value[0] === value[1]) {
result[keyLower] = value[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

are we sure they are always exactly 2 values in the array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are we sure they are always exactly 2 values in the array?

Yeah, and it could only happen if it returns null from a get in the cacheHandler.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can add the info in the JS doc ?

} else {
logger.warn(
"Multiple different values for Location header found. Using the last one",
);
result[keyLower] = value[value.length - 1];
}
continue;
}
result[keyLower] = convertHeader(value);
}

return result;
Expand Down
25 changes: 24 additions & 1 deletion packages/tests-unit/tests/http/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { parseSetCookieHeader } from "@opennextjs/aws/http/util.js";
import type http from "node:http";

import {
parseHeaders,
parseSetCookieHeader,
} from "@opennextjs/aws/http/util.js";

describe("parseSetCookieHeader", () => {
it("returns an empty list if cookies is emptyish", () => {
Expand Down Expand Up @@ -43,3 +48,21 @@ describe("parseSetCookieHeader", () => {
]);
});
});

describe("parseHeaders", () => {
it("parses headers correctly", () => {
const headers = parseHeaders({
location: ["/target", "/target"],
"x-custom-header": "customValue",
"x-multiple-values": ["value1", "value2"],
"x-undefined-header": undefined,
"x-opennext": "is-so-cool",
} as unknown as http.OutgoingHttpHeaders);
expect(headers).toEqual({
location: "/target",
"x-custom-header": "customValue",
"x-multiple-values": "value1,value2",
"x-opennext": "is-so-cool",
});
});
});
Loading