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

个��.hk throws error at Object.toASCII (ext:deno_node/punycode.ts:5:16) #19214

Closed
tigitz opened this issue May 22, 2023 · 0 comments · Fixed by #22847
Closed

个��.hk throws error at Object.toASCII (ext:deno_node/punycode.ts:5:16) #19214

tigitz opened this issue May 22, 2023 · 0 comments · Fixed by #22847
Assignees
Labels
bug Something isn't working correctly node API Related to various "node:*" modules APIs node compat

Comments

@tigitz
Copy link

tigitz commented May 22, 2023

Following #19113

Usage example still fails but now with a new error:

ERROR CheerioCrawler: Request failed and reached maximum retries. Error: Errors { invalid_mapping, disallowed_character }
    at Object.toASCII (ext:deno_node/punycode.ts:5:16)
    at file:///home/username/.cache/deno/npm/registry.npmjs.org/psl/1.9.0/index.js:47:34
    at Array.reduce (<anonymous>)
    at Object.internals.findRule (file:///home/username/.cache/deno/npm/registry.npmjs.org/psl/1.9.0/index.js:41:26)
    at Object.exports.parse (file:///home/username/.cache/deno/npm/registry.npmjs.org/psl/1.9.0/index.js:201:24)
    at Object.exports.get (file:///home/username/.cache/deno/npm/registry.npmjs.org/psl/1.9.0/index.js:261:18)
    at Object.getPublicSuffix (file:///home/username/.cache/deno/npm/registry.npmjs.org/tough-cookie/4.1.2/lib/pubsuffix-psl.js:70:14)
    at permuteDomain (file:///home/username/.cache/deno/npm/registry.npmjs.org/tough-cookie/4.1.2/lib/permuteDomain.js:38:28)
    at MemoryCookieStore.findCookies (file:///home/username/.cache/deno/npm/registry.npmjs.org/tough-cookie/4.1.2/lib/memstore.js:99:21)
    at MemoryCookieStore.findCookies (file:///home/username/.cache/deno/npm/registry.npmjs.org/universalify/0.2.0/index.js:5:67)

Code where it fails: https://github.com/lupomontero/psl/blob/main/index.js#L44

I've tracked it down to the 个��.hk string that is passed.

Surprisingly, following works:

import * as mod from "npm:punycode";
console.log(mod.default.toASCII('个��.hk'));

Deno version is built from source from this commit: 40bda07

@bartlomieju bartlomieju added bug Something isn't working correctly node compat labels May 22, 2023
@bartlomieju bartlomieju added the node API Related to various "node:*" modules APIs label Mar 4, 2024
nathanwhit added a commit that referenced this issue Mar 11, 2024
Fixes #19214.

We were using the `idna` crate to implement our polyfill for
`punycode.toASCII` and `punycode.toUnicode`. The `idna` crate is
correct, and adheres to the IDNA2003/2008 spec, but it turns out
`node`'s implementations don't really follow any spec! Instead, node
splits the domain by `'.'` and punycode encodes/decodes each part. This
means that node's implementations will happily work on codepoints that
are disallowed by the IDNA specs, causing the error in #19214.

While fixing this, I went ahead and matched the node behavior on all of
the punycode functions and enabled node's punycode test in our
`node_compat` suite.
nathanwhit added a commit that referenced this issue Mar 14, 2024
Fixes #19214.

We were using the `idna` crate to implement our polyfill for
`punycode.toASCII` and `punycode.toUnicode`. The `idna` crate is
correct, and adheres to the IDNA2003/2008 spec, but it turns out
`node`'s implementations don't really follow any spec! Instead, node
splits the domain by `'.'` and punycode encodes/decodes each part. This
means that node's implementations will happily work on codepoints that
are disallowed by the IDNA specs, causing the error in #19214.

While fixing this, I went ahead and matched the node behavior on all of
the punycode functions and enabled node's punycode test in our
`node_compat` suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node API Related to various "node:*" modules APIs node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants