Sitelet https://web.archive.org/web/20201023140658/https://github.com/nodejs/node/issues/35688
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

%.* is not a supported debug specifier #35688

Open
mmomtchev opened this issue Oct 16, 2020 · 3 comments · May be fixed by #35694
Open

%.* is not a supported debug specifier #35688

mmomtchev opened this issue Oct 16, 2020 · 3 comments · May be fixed by #35694
Labels

Comments

@mmomtchev
Copy link

@mmomtchev mmomtchev commented Oct 16, 2020

  • Version: v14.13.1
  • Platform: Linux
  • Subsystem: http2

What steps will reproduce the bug?

(example from #35475)
NODE_DEBUG_NATIVE=HTTP2SESSION node

const http2 = require('http2');
const tls = require('tls');
const stream = require('stream');

const options = {
    ALPNProtocols: ['h2'],
    host: 'nghttp2.org',
    servername: 'nghttp2.org',
    port: 443
};

const socket = tls.connect(options, async () => {
    console.log('TLS Connected!');

    const proxy = new stream.Duplex({
        read(...args) {
            const x = socket.read();
            console.log('read', JSON.stringify(x));
            if (x !== null)
            this.push(x);
        },

        final(...args) {
            socket.end(...args);
        },

        write(...args) {
            socket.write(...args);
        },

        destroy(...args) {
            socket.destroy(...args);
        }
    });

    await new Promise(resolve => setTimeout(resolve, 2000));

    const session = http2.connect('https://nghttp2.org', {
        createConnection: () => socket
    });

    session.once('remoteSettings', () => {
        console.log('HTTP2 Received remote settings!');
    });

    session.once('connect', () => {
        console.log('HTTP2 Connected!');
    });

});

Additional information

master is not affected

@mmomtchev
Copy link
Author

@mmomtchev mmomtchev commented Oct 16, 2020

switch (*p) {
doesn't seem to support %.* as a format specifier, yet there are a few examples in the code? @addaleax?
Currently the % eats the length specifier then the second argument triggers the CHECK_NOT_NULL

@mmomtchev mmomtchev changed the title Segfault with NODE_DEBUG_NATIVE=HTTP2SESSION %.* is not a supported debug specifier Oct 17, 2020
@addaleax
Copy link
Member

@addaleax addaleax commented Oct 17, 2020

@mmomtchev Yeah, length specifiers make sense for a printf that is mainly intended to handle C strings, but it’s not important for a type-aware C++ variant like the one that we have. I think it would be just fine here to replace Debug(session, "Error '%.*s'", len, message); with Debug(session, "Error '%s'", std::string(message, len));.

@mmomtchev
Copy link
Author

@mmomtchev mmomtchev commented Oct 17, 2020

Yes, in fact, there are 3 different sprintfs used through out the code: this one, from debug_utils-inl.h, the V8 one and the OS-provided one - and all the other examples are for the other two which support it

mmomtchev added a commit to mmomtchev/node that referenced this issue Oct 17, 2020
The debug sprintf doesn't support %.* specifiers

Fixes: nodejs#35688
@mmomtchev mmomtchev linked a pull request that will close this issue Oct 17, 2020
2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.