Repository navigation
Migration from process.binding #22064
Description
Activity
i have reservations about exposing anything on process.binding('util') publicly... stack decoration shouldn't have a node-specific solution and promise and proxy details can't be equally represented by the engines that might want to run node.
taking a step back, i think we should look at how the things these apis enabled would be implemented if these apis had never existed, instead of straight-up replacements.
stack decoration shouldn't have a node-specific solution
Cool. As long as there's some way, it would be nice. (#21958 is a step to providing a way)
and promise and proxy details can't be equally represented by the engines that might want to run node.
Right, Chakra had to do extra work to support those.
taking a step back, i think we should look at how the things these apis enabled would be implemented if these apis had never existed, instead of straight-up replacements.
👏 That's a great exercise!
consoleCall (inspector wiring)
@jdalton have you seen https://nodejs.org/api/inspector.html#inspector_inspector_console to print messages into the inspector console?
have you seen https://nodejs.org/api/inspector.html#inspector_inspector_console to print messages into the inspector console?
Yes. I believe you're saying I can create my own form of
consoleCallthat pipes to Node's console and the inspector console with the exposedoriginalConsole. 👍It would be great if you could confirm that, so we can rule out one item from that list.
I am not sure that I am not missing some context, but the vital part of console.log behavior at least on inspector side - it is providing stack trace that points to the place where console.log was called. I am worried that with plain JS function we will get JS stack frame that contains location inside this function.
node/lib/internal/bootstrap/node.js
Lines 441 to 444 in ce98e2e
right here the two functions are wrapped to be called by a native function (consoleCall is c++)
Lines 157 to 181 in ce98e2e
@jdalton you can try replacing the c++ consoleCall with this if you wanna play around with it.
// because this is written in js it will be added as an additional frame // which doesn't show up in the c++ version function consoleCall(inspector_method, node_method, config_object, ...args) { const in_call_key = 'in_call'; if (!(in_call_key in config_object)) { config_object[in_call_key] = true; inspector_method(...args); delete config_object[in_call_key]; } node_method(...args); }
Test script:
'use strict'; const { Console } = require('console'); const originalConsole = require('inspector').console; const config = {}; // Pretty straightforward port of InspectorConsoleCall() to JavaScript. function consoleCall(inspectorMethod, nodeMethod, config, ...args) { // Assume inspector is always on. if (!('in_call' in config)) { config.in_call = true; inspectorMethod.apply(this, args); } delete config.in_call; nodeMethod.apply(this, args); } const jsWrappedConsole = new Console(process.stdout); jsWrappedConsole.error = consoleCall.bind(jsWrappedConsole, originalConsole.error, jsWrappedConsole.error, config); function usesCPPWrapped() { console.error('bleh'); } function usesJSWrapped() { jsWrappedConsole.error('bleh'); } usesCPPWrapped(); usesJSWrapped();
Note how
console.error's stack is at the right place, butjsWrappedConsole.error's error symbol is always on theinspectorMethod.applyline.Hope this helps.
Reacted by snek, John-David Dalton and AlexeyReacted by John-David DaltonAh yep okay, since
console.errordoesn't actually throw I can't get creative withError.captureStackTraceeither.@jdalton you can do
builtinLibs.includes('inspector')for thatopenmethod which would be much more idiomatic anyway.if my changes to error stack decoration ever land there will be no need for
decorated_private_symbol,setHiddenValue, andgetHiddenValue.i would recommend creating os.getenv and os.setenv as a replacement for safeGetenv.
i would strongly disagree with exposing proxy inspection as an api and i will to the best of my ability block exposing promise inspection
13 remaining items
Is #3591 related to this at all? It still has not been resolved.
To be honest, I don't really know @stevenvachon, it's been a while since I've looked that issue over.
As @bnoordhuis points out in the thread in #21509, the current behavior is intentional as to avoid a very real security vulnerability, and it is working as intended.
What is the security vulnerability? He doesn't state in that comment the implications.
What is the workaround for when a remote service returns invalid response, such as in a scenario I describe in #22064 (comment) ?
Strict handling of invalid characters is necessary to prevent a variety of request smuggling and response splitting style attacks.
The correct workaround is to try to get the broken remote service fixed. Not sure what else we can do now that the legacy parser has been removed.
was the question here about loosening how llhttp works or allowing the parser to be overridden? the latter seems like a reasonable request to me.
The correct workaround is to try to get the broken remote service fixed. Not sure what else we can do now that the legacy parser has been removed.
That is an insane position to take.
I run a large data aggregation network. There are literally hundreds of websites misbehaving even within our relatively restricted domain operation.
This change literally makes Node.js not usable for us.
Reacted by Joseph Madden, Matt Tucker, Jacob Beard and Chris@gajus Although I see the relationship, I feel like this is turning into a separate discussion from what the issue is about, and I’d suggest opening a new issue, ideally with a description of what exact “features” you need from the newer parser that the old one had.
allowing the parser to be overridden
This has been discussed in the past (repeatedly and at length) and the answer has always been 'no' because it turns too many implementation details into frozen-forever public API.
Custom JS parsers exist (link). You could even WASM-ify http-parser if you want bug-for-bug compatibility. I'm its maintainer and I'd be open to that.
For all internal uses, we have migrated away from using
process.binding(). I believe we can close this issue now, although there is the follow on question of when/how we can deprecateprocess.binding()
With #22004 landed to doc deprecate
process.bindingI was encouraged to open an issue related to my use of it. Now is a great time to starting examining what fromprocess.bindingcan be exposed in a user-friendly way.In my own experience I've attempted to use (if available and there were no user-facing options) these bindings. My usage fits into these categories
The API run down as follows:
process.binding("config")experimentalREPLAwait (command line checks)experimentalWorker (command line checks)exposeInternals (command line checks)preserveSymlinks (command line checks)preserveSymlinksMain (command line checks)process.binding("inspector")open (only existence checks, much like Node does)process.binding("util")For chrome inspector wiring there has been some work to expose things like
originalConsole(#21659), but more in this area is needed.For command-line checks the answer may be to use
process.execArgv, but a moreconfiglike object form would be handy.For custom error stack decoration there is early feelers in #21958.
Custom inspection helpers like
getProxyDetails/getPromiseDetailsandsafeGetenvhave nothing simmering at the moment.Updated:
I
crossed throughAPI that have user-facing options.Update:
Replaced
process.binding("inspector").openinferences withprocess.config.variables.v8_enable_inspectorchecks.