util: util.debuglog() is buggy and poorly documented

[vsemozhetbyt]

• Version: possibly all actual ones
• Platform: possibly all
• Subsystem: util

Doc says nothing about the acceptable format of the section strings. However, the check in the code has some silent restrictions/prerequisites:

1. User input is not escaped, so if it contains unintentional RegExp special characters, things become unpredictable.
2. Doc states the delimiters to be commas, but the code checks borders via \b symbol (word boundary) and this can make a mess: a section with non-[a-z0-9_] symbols at the beginning or the end becomes always unmatched, while a multiword section may trigger some unintended matchings.

This is prone to false positive/false negative effects and crashes. For example:

const util = require('util');

const debuglogMustCall_1 = util.debuglog('###');
const debuglogMustCall_2 = util.debuglog('f$oo');

const debuglogMustNotCall_1 = util.debuglog('f.oo');
const debuglogMustNotCall_2 = util.debuglog('bar');

debuglogMustCall_1('this should be logged');
debuglogMustCall_2('this should be logged too');

debuglogMustNotCall_1('this should not be logged');
debuglogMustNotCall_2('this should not be logged too');

> SET NODE_DEBUG=###,f$oo,no-bar-at-all
> node test.js

F.OO 5604: this should not be logged
BAR 5604: this should not be logged too

const util = require('util');

const debuglogTHROWS = util.debuglog('hi:)');

debuglogTHROWS('hi there!');

> SET NODE_DEBUG=hi:)
> node test.js

util.js:147
    if (new RegExp(`\\b${set}\\b`, 'i').test(debugEnviron)) {
        ^

SyntaxError: Invalid regular expression: /\bHI:)\b/: Unmatched ')'

What possibly could be done:

1. Document this weird situation (like 'only one word [a-z0-9_] strings are allowed' or something).
2. Or change the code: split the NODE_DEBUG value by commas and check via sections.map(toUpperCase).includes(set.toUpperCase()). This may be semver-major unfortunately.

[vsemozhetbyt]

It seems we have a test for only one simple section variant.

[vsemozhetbyt]

cc @bnoordhuis, @cjihrig, @evanlucas
Should we address this issue? If so, what way should we prefer?

[evanlucas]

My understanding is that this was/is meant only for use inside core, so I'm not sure we should do anything about it?

[vsemozhetbyt]

As it is documented without any precautions, it seems this is for userland debug usage as well.