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

Argument is not a ByteString when setting headers with non-ASCII characters #1590

Closed
benmccann opened this issue Aug 3, 2022 · 5 comments · Fixed by #1591
Closed

Argument is not a ByteString when setting headers with non-ASCII characters #1590

benmccann opened this issue Aug 3, 2022 · 5 comments · Fixed by #1591
Labels
bug Something isn't working

Comments

@benmccann
Copy link
Contributor

Bug Description

It looks like #1317 wasn't actually fixed

Reproducible By

I got this while setting the link header to the following value:

</_app/immutable/assets/__layout-a4ea7e9d.css>; rel="stylesheet"; nopush,</_app/immutable/start-cf8d1dd9.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/index-bf47b8b5.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/paths-98950022.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/preload-helper-c28b9807.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/singletons-1e28953d.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/env-public-887c86aa.js>; rel="modulepreload"; nopush,</_app/immutable/pages/__layout.svelte-b3b95660.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/navigation-c4179065.js>; rel="modulepreload"; nopush,</_app/immutable/pages/encoded/苗条.svelte-7e7988f6.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/stores-2e0a9bb3.js>; rel="modulepreload"; nopush

Expected Behavior

I think this should work. If it's invalid in some way, the error message should certainly be clarified. I passed a string which is valid, so it shouldn't be complaining about wanting a ByteString

Logs & Screenshots

TypeError: Argument is not a ByteString
    at Object.webidl.converters.ByteString (node_modules/.pnpm/undici@5.8.1/node_modules/undici/lib/fetch/webidl.js:404:11)
    at Headers.set (node_modules/.pnpm/undici@5.8.1/node_modules/undici/lib/fetch/headers.js:362:31)
    at render_response (packages/kit/test/apps/basics/.svelte-kit/output/server/_app/immutable/chunks/index-fb484aa3.js:1090:13)
    at respond$1 (packages/kit/test/apps/basics/.svelte-kit/output/server/_app/immutable/chunks/index-fb484aa3.js:2013:25)
    at resolve (packages/kit/test/apps/basics/.svelte-kit/output/server/_app/immutable/chunks/index-fb484aa3.js:2261:114)
    at packages/kit/test/apps/basics/.svelte-kit/output/server/_app/immutable/chunks/hooks-a33195ff.js:44:20
    at respond (packages/kit/test/apps/basics/.svelte-kit/output/server/_app/immutable/chunks/index-fb484aa3.js:2213:22)
    at packages/kit/src/vite/preview/index.js:142:5

Environment

Undici 5.8.1
Node 16.15.0
Ubuntu 22.04

Additional context

This is blocking the SvelteKit web framework from setting the link header

@benmccann benmccann added the bug Something isn't working label Aug 3, 2022
@ronag
Copy link
Member

ronag commented Aug 3, 2022

@KhafraDev

@KhafraDev
Copy link
Member

This behavior is expected, you can test it for yourself in the browser console:

const headers = new Headers()
headers.set('link', '</_app/immutable/assets/__layout-a4ea7e9d.css>; rel="stylesheet"; nopush,</_app/immutable/start-cf8d1dd9.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/index-bf47b8b5.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/paths-98950022.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/preload-helper-c28b9807.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/singletons-1e28953d.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/env-public-887c86aa.js>; rel="modulepreload"; nopush,</_app/immutable/pages/__layout.svelte-b3b95660.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/navigation-c4179065.js>; rel="modulepreload"; nopush,</_app/immutable/pages/encoded/苗条.svelte-7e7988f6.js>; rel="modulepreload"; nopush,</_app/immutable/chunks/stores-2e0a9bb3.js>; rel="modulepreload"; nopush')

If it's invalid in some way, the error message should certainly be clarified.

Agreed, the errors around the webidl stuff should be improved.

I passed a string which is valid, so it shouldn't be complaining about wanting a ByteString

A ByteString is not a JS string; it can only contain ASCII and 8-bit characters (char codes between 128 - 255). The reason this string is throwing an error is because of the characters 苗条.

The webidl for the headers class indicates for implementations to convert both parameters of Headers.prototype.set to ByteStrings.


For more info, see:

@KhafraDev
Copy link
Member

I'm going to re-open this until there are better error messages

@benmccann
Copy link
Contributor Author

Thanks for the clarification! The improved error will help some. I'd probably still be confused that it was trying to convert it to a ByteString as lib.dom.d.ts says value: string and MDN didn't mention this restriction either. I guess that's outside the realm of this repo, but maybe I'll try to send a PR to MDN at least

@benmccann
Copy link
Contributor Author

I've updated MDN: mdn/content#19166

I'll leave updating TypeScript's lib.dom.d.ts for someone else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants