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 multipart boundary mismatch #540

Merged
merged 3 commits into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
- run: npm install
- name: Install Playwright Browsers
run: npx playwright install --with-deps
- run: npm test
# TODO: Enable again when https://github.com/istanbuljs/nyc/issues/1287 is fixed.
# - uses: codecov/codecov-action@v1
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"expect-type": "^0.16.0",
"express": "^4.18.2",
"pify": "^6.1.0",
"playwright-chromium": "^1.37.1",
"playwright": "^1.39.0",
"raw-body": "^2.5.2",
"ts-node": "^10.9.1",
"typescript": "^5.2.2",
Expand Down
19 changes: 19 additions & 0 deletions source/core/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {Expect, Equal} from '@type-challenges/utils';
import {type HttpMethod, type KyOptionsRegistry} from '../types/options.js';
import {type RequestInitRegistry} from '../types/request.js';

export const supportsRequestStreams = (() => {
let duplexAccessed = false;
Expand Down Expand Up @@ -58,3 +59,21 @@ export const kyOptionKeys: KyOptionsRegistry = {
onDownloadProgress: true,
fetch: true,
};

export const requestOptionsRegistry: RequestInitRegistry = {
method: true,
headers: true,
body: true,
mode: true,
credentials: true,
cache: true,
redirect: true,
referrer: true,
referrerPolicy: true,
integrity: true,
keepalive: true,
signal: true,
window: true,
dispatcher: true,
duplex: true,
};
5 changes: 5 additions & 0 deletions source/types/request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import {type RequestInit as UndiciRequestInit} from 'undici-types';

type CombinedRequestInit = globalThis.RequestInit & UndiciRequestInit;

export type RequestInitRegistry = {[K in keyof CombinedRequestInit]-?: true};
4 changes: 2 additions & 2 deletions source/utils/options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {kyOptionKeys} from '../core/constants.js';
import {kyOptionKeys, requestOptionsRegistry} from '../core/constants.js';

export const findUnknownOptions = (
request: Request,
Expand All @@ -7,7 +7,7 @@ export const findUnknownOptions = (
const unknownOptions: Record<string, unknown> = {};

for (const key in options) {
if (!(key in kyOptionKeys) && !(key in request)) {
if (!(key in requestOptionsRegistry) && !(key in kyOptionKeys) && !(key in request)) {
unknownOptions[key] = options[key];
}
}
Expand Down
32 changes: 15 additions & 17 deletions test/browser.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import test, {type ExecutionContext} from 'ava';
import busboy from 'busboy';
import express from 'express';
import {type Page} from 'playwright-chromium';
import {chromium, webkit, type Page} from 'playwright';
import type ky from '../source/index.js';
import {createHttpTestServer, type ExtendedHttpTestServer, type HttpServerOptions} from './helpers/create-http-test-server.js';
import {parseRawBody} from './helpers/parse-body.js';
import {withPage} from './helpers/with-page.js';
import {browserTest, defaultBrowsersTest} from './helpers/with-page.js';

declare global {
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
Expand Down Expand Up @@ -46,7 +46,7 @@ test.afterEach(async () => {
await server.close();
});

test.serial('prefixUrl option', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('prefixUrl option', async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end('zebra');
});
Expand Down Expand Up @@ -74,7 +74,7 @@ test.serial('prefixUrl option', withPage, async (t: ExecutionContext, page: Page
t.deepEqual(results, ['rainbow', 'rainbow', 'rainbow', 'rainbow']);
});

test.serial('aborting a request', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('aborting a request', async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end('meow');
});
Expand All @@ -98,7 +98,7 @@ test.serial('aborting a request', withPage, async (t: ExecutionContext, page: Pa
t.is(errorName, 'AbortError');
});

test.serial('should copy origin response info when using `onDownloadProgress`', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('should copy origin response info when using `onDownloadProgress`', async (t: ExecutionContext, page: Page) => {
const json = {hello: 'world'};
const status = 202;
const statusText = 'Accepted';
Expand Down Expand Up @@ -133,7 +133,7 @@ test.serial('should copy origin response info when using `onDownloadProgress`',
});
});

test.serial('should not copy response body with 204 status code when using `onDownloadProgress` ', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('should not copy response body with 204 status code when using `onDownloadProgress` ', async (t: ExecutionContext, page: Page) => {
const status = 204;
const statusText = 'No content';
server.get('/', (_request, response) => {
Expand Down Expand Up @@ -182,7 +182,7 @@ test.serial('should not copy response body with 204 status code when using `onDo
}]);
});

test.serial('aborting a request with onDonwloadProgress', withPage, async (t: ExecutionContext, page: Page) => {
browserTest('aborting a request with onDownloadProgress', [chromium], async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end('meow');
});
Expand Down Expand Up @@ -214,9 +214,8 @@ test.serial('aborting a request with onDonwloadProgress', withPage, async (t: Ex
t.is(error, 'TypeError: Failed to fetch');
});

test.serial(
defaultBrowsersTest(
'throws TimeoutError even though it does not support AbortController',
withPage,
async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end();
Expand Down Expand Up @@ -249,7 +248,7 @@ test.serial(
},
);

test.serial('onDownloadProgress works', withPage, async (t: ExecutionContext, page: Page) => {
browserTest('onDownloadProgress works', [chromium, webkit], async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.writeHead(200, {
'content-length': '4',
Expand Down Expand Up @@ -289,7 +288,7 @@ test.serial('onDownloadProgress works', withPage, async (t: ExecutionContext, pa
t.is(result.text, 'meow');
});

test.serial('throws if onDownloadProgress is not a function', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('throws if onDownloadProgress is not a function', async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end();
});
Expand All @@ -305,7 +304,7 @@ test.serial('throws if onDownloadProgress is not a function', withPage, async (t
t.is(error, 'TypeError: The `onDownloadProgress` option must be a function');
});

test.serial('throws if does not support ReadableStream', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('throws if does not support ReadableStream', async (t: ExecutionContext, page: Page) => {
server.get('/', (_request, response) => {
response.end();
});
Expand All @@ -322,7 +321,7 @@ test.serial('throws if does not support ReadableStream', withPage, async (t: Exe
t.is(error, 'Error: Streams are not supported in your environment. `ReadableStream` is missing.');
});

test.serial('FormData with searchParams', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('FormData with searchParams', async (t: ExecutionContext, page: Page) => {
t.plan(3);

server.get('/', (_request, response) => {
Expand Down Expand Up @@ -354,7 +353,7 @@ test.serial('FormData with searchParams', withPage, async (t: ExecutionContext,
}, server.url);
});

test.serial('FormData with searchParams ("multipart/form-data" parser)', withPage, async (t: ExecutionContext, page: Page) => {
defaultBrowsersTest('FormData with searchParams ("multipart/form-data" parser)', async (t: ExecutionContext, page: Page) => {
t.plan(3);

server.get('/', (_request, response) => {
Expand Down Expand Up @@ -426,9 +425,8 @@ test.serial('FormData with searchParams ("multipart/form-data" parser)', withPag
}, server.url);
});

test.serial(
defaultBrowsersTest(
'headers are preserved when input is a Request and there are searchParams in the options',
withPage,
async (t: ExecutionContext, page: Page) => {
t.plan(2);

Expand Down Expand Up @@ -459,7 +457,7 @@ test.serial(
},
);

test.serial('retry with body', withPage, async (t: ExecutionContext, page: Page) => {
browserTest('retry with body', [chromium, webkit], async (t: ExecutionContext, page: Page) => {
t.plan(4);

let requestCount = 0;
Expand Down
31 changes: 20 additions & 11 deletions test/helpers/with-page.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
/* eslint-disable ava/no-ignored-test-files */
import process from 'node:process';
import type {ExecutionContext, Implementation} from 'ava';
import {chromium, type Page} from 'playwright-chromium';
import test from 'ava';
import {chromium, firefox, webkit, type BrowserType, type Page} from 'playwright';
import type {ExecutionContext} from 'ava';

type Run = (t: ExecutionContext, page: Page) => Promise<void>;

const PWDEBUG = Boolean(process.env['PWDEBUG']);
const DEFAULT_BROWSERS = [chromium, firefox, webkit];

export const withPage: Implementation<any[]> = async (t: ExecutionContext, run: Run): Promise<void> => {
const browser = await chromium.launch({
devtools: PWDEBUG,
});
const page = await browser.newPage();
try {
await run(t, page);
} finally {
await browser.close();
export const browserTest = (title: string, browserTypes: BrowserType[], run: Run) => {
for (const browserType of browserTypes) {
test.serial(`${browserType.name()} - ${title}`, async t => {
const browser = await browserType.launch({devtools: PWDEBUG});
const page = await browser.newPage();
try {
await run(t, page);
} finally {
await browser.close();
}
});
}
};

export const defaultBrowsersTest = (title: string, run: Run) => {
browserTest(title, DEFAULT_BROWSERS, run);
};