foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-23 07:44:48 -05:00
Code Issues Packages 1 Wiki Activity
denoland-deno/ext/node
History
Max Goodhart 2235a1a359
fix(node/tls): fix NotValidForName for host set via socket / servername (#21441) 
This PR is an attempt to fix
https://github.com/denoland/deno/issues/20293, in which node modules
connecting to databases fail due to TLS errors. I ran into this
attempting to use
[node-postgres](https://github.com/brianc/node-postgres) to connect to a
[Neon](https://neon.tech) database.

Investigating via `--inspect-brk` led me to notice that the hostname
eventually passed to `Deno.startTls` was null. The hostname is
determined by the following code:


f6b889b432/ext/node/polyfills/_tls_wrap.ts (L87-L89)

This logic doesn't appear to be correct. I couldn't find reference to
`servername` existing on the `secureContext` in either Node's or Deno's
docs. There's a lot of scope here, and it's my first time reading
through this code, so I could be missing something!

Node uses [the following
logic](2e458d9736/lib/_tls_wrap.js (L1679-L1682)
) to determine the hostname for certificate validation:
 
```
    const hostname = options.servername ||
                   options.host ||
                   (options.socket && options.socket._host) ||
                   'localhost';
```

This PR updates the `TLSSocket` polyfill to use behave similarly (though
I omitted the default to `localhost` at the end; I'm not sure if
including it is necessary or correct). With this change, `node-postgres`
connects to my TLS endpoint successfully (aside: Neon requires SNI,
which also works as expected).

---

I tried to update the tests in
https://github.com/denoland/deno/blob/main/cli/tests/unit_node/tls_test.ts
to exercise this change, but the test fails for me on `main` on Linux. I
investigated briefly and noticed that the test fixture
`cli/tests/testdata/tls/localhost.crt` doesn't appear to include the
`subjectAltName` specified in `domains.txt`. I believe the certificate
isn't matching `localhost`, but that's where I ended investigating.
2023-12-08 03:53:36 +00:00
..
ops perf(node/fs): faster existsSync when not exists (#21458) 2023-12-04 21:05:40 +00:00
polyfills fix(node/tls): fix NotValidForName for host set via socket / servername (#21441) 2023-12-08 03:53:36 +00:00
analyze.rs fix(node): cjs export analysis should probe for json files (#21113) 2023-11-07 16:38:55 -05:00
build.rs build: allow disabling snapshots for dev (#20048) 2023-08-06 01:47:15 +02:00
Cargo.toml chore: forward v1.38.5 release commit to main (#21472) 2023-12-06 00:53:16 +00:00
clippy.toml refactor(ext/fs): deno_fs::FileSystem - conditional Send + Sync (#18993) 2023-05-08 11:02:02 -04:00
errors.rs fix(node): inspect ancestor directories when resolving cjs re-exports during analysis (#21104) 2023-11-07 09:56:06 -05:00
global.rs fix(ext/node): include non-enumerable keys in Reflect.ownKeys(globalThis) (#21485) 2023-12-07 21:34:44 +05:30
lib.rs perf(node/fs): faster existsSync when not exists (#21458) 2023-12-04 21:05:40 +00:00
package_json.rs fix(node): inspect ancestor directories when resolving cjs re-exports during analysis (#21104) 2023-11-07 09:56:06 -05:00
path.rs chore(ext/node): fix variable name (#17948) 2023-02-27 21:26:02 +01:00
polyfill.rs fix(node): repl._builtinLibs (#20046) 2023-08-04 14:30:48 +02:00
README.md chore(ext/node): correct publishing for ext/node (#15461) 2022-08-11 17:25:41 -04:00
resolution.rs fix(node): use closest package.json to resolve package.json imports (#21075) 2023-11-04 16:41:51 +00:00

README.md

deno_node

require and other node related functionality for Deno.