Skip to content
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(cookies): Update Undici to 5.20 and fix cookies behaviour #6323

Merged
merged 2 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
"rollup": "^3.9.0",
"sass": "^1.52.2",
"srcset-parse": "^1.1.0",
"undici": "^5.14.0",
"undici": "^5.20.0",
"unified": "^10.1.2"
},
"engines": {
Expand Down
6 changes: 0 additions & 6 deletions packages/astro/src/vite-plugin-astro-server/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ export async function writeWebResponse(res: http.ServerResponse, webResponse: Re

const _headers = Object.fromEntries(headers.entries());

// Undici 5.19.1 includes a `getSetCookie` helper that returns an array of all the `set-cookies` headers.
// Previously, `headers.entries()` would already have those merged, but it seems like this isn't the case anymore, weird.
if ((headers as any)['getSetCookie']) {
_headers['set-cookie'] = (headers as any).getSetCookie();
}

// Attach any set-cookie headers added via Astro.cookies.set()
const setCookieHeaders = Array.from(getSetCookiesFromResponse(webResponse));
if (setCookieHeaders.length) {
Expand Down
33 changes: 5 additions & 28 deletions packages/integrations/netlify/src/netlify-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,34 +102,11 @@ export const createExports = (manifest: SSRManifest, args: Args) => {
isBase64Encoded: responseIsBase64Encoded,
};

// Special-case set-cookie which has to be set an different way :/
// The fetch API does not have a way to get multiples of a single header, but instead concatenates
// them. There are non-standard ways to do it, and node-fetch gives us headers.raw()
// See https://github.com/whatwg/fetch/issues/973 for discussion
if (response.headers.has('set-cookie')) {
if ('raw' in response.headers) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Old left over from node-fetch. When everything is gonna be updated to use getSetCookie, we'll also be able to remove the whole splitCookiesString thing

// Node fetch allows you to get the raw headers, which includes multiples of the same type.
// This is needed because Set-Cookie *must* be called for each cookie, and can't be
// concatenated together.
type HeadersWithRaw = Headers & {
raw: () => Record<string, string[]>;
};

const rawPacked = (response.headers as HeadersWithRaw).raw();
if ('set-cookie' in rawPacked) {
fnResponse.multiValueHeaders = {
'set-cookie': rawPacked['set-cookie'],
};
}
} else {
const cookies = response.headers.get('set-cookie');

if (cookies) {
fnResponse.multiValueHeaders = {
'set-cookie': Array.isArray(cookies) ? cookies : splitCookiesString(cookies),
};
}
}
const cookies = response.headers.get('set-cookie');
if (cookies) {
fnResponse.multiValueHeaders = {
'set-cookie': Array.isArray(cookies) ? cookies : splitCookiesString(cookies),
};
}

// Apply cookies set via Astro.cookies.set/delete
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@
"cheerio": "^1.0.0-rc.11",
"mocha": "^9.2.2",
"node-mocks-http": "^1.11.0",
"undici": "^5.14.0"
"undici": "^5.20.0"
}
}
2 changes: 1 addition & 1 deletion packages/telemetry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"dset": "^3.1.2",
"is-docker": "^3.0.0",
"is-wsl": "^2.2.0",
"undici": "^5.14.0",
"undici": "^5.20.0",
"which-pm-runs": "^1.1.0"
},
"devDependencies": {
Expand Down
19 changes: 13 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.