Skip to content

Commit

Permalink
Fix multipart boundary mismatch (#540)
Browse files Browse the repository at this point in the history
  • Loading branch information
kdelmonte committed Oct 29, 2023
1 parent dd713b6 commit 0e9d7bb
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 31 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
};

0 comments on commit 0e9d7bb

Please sign in to comment.