Fix: Invalid HTTP/2 origin set when servername is empty #39919
The bug posted by @szmarczak is because of not checking the truthy value of options.servername in line 3101 and 3102 of lib/internal/http2/core.js, instead the options.servername is strictly checked against undefined, any value other than undefined will give undesirable result in this case.
So in this PR,
if (servername !== undefined && options.servername === undefined)
options.servername = servername;
is changed to,
if (servername !== undefined && !options.servername)
options.servername = servername;
Here, the truthy value of options.servername is checked rather than strict check against undefined.
I have built node.js locally and checked against the code posted by @szmarczak to reproduce the bug.
Could we add a test for the issue?
@Ayase-252 yeah that would be great. I'll look into how tests are organised in this project so I can write few cases.
@Ayase-252 I have added test cases.
@Narasimha1997 Lint fails. The commit message check fails too.
Commit message: https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines. Naming test files: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#naming-test-files.
@Mesteery @szmarczak @Ayase-252 Well, haha. Last time I made a contribution to Node an year ago, this commit message linting was not there. So what would you suggest? Can you please give an example of how issue fix commit message must be formulated?
@Narasimha1997
Maybe
http2: set origin name correctly when servername is empty
Fixes: https://github.com/nodejs/node/issues/39919
The build on windows has failed due to some issues while fetching NASM. Can we re-run the build? Now it seems to be working fine.