Crash if server receives an invalid method name

[crystalfp]

I use spdy module to provide http2 for my express API server.

"use strict";

/* > Load required modules */
const express = require("express");
const spdy = require("spdy");
const fs = require("fs");

/* > Initialize application */
const app = express();

/* > Endpoint "kb" */
app.get("/api/kb", function(req, res) {res.send("All OK\n");});

if(false) {
	// This does not crash (curl returns "empty reply from server")
	app.listen(9999, () => console.log(`\nServer started\n`));
}
else {
	// This crashes
	const certDir = __dirname + "/js/cert";
	const options = {
		key: fs.readFileSync(certDir + "/server.key"),
		cert: fs.readFileSync(certDir + "/server.crt")
	};

	spdy
		.createServer(options, app)
		.listen(9999, (error) => {
			if(error) {
				console.error(error);
				throw error;
			}
			else {
				console.log(`\nServer started (HTTP/2)\n`);
			}
		});
}

I access the API with the following curl command:

C:/apps/curl/bin/curl.exe -k -X INVALID https://localhost:9999/api/kb

Note that I access the API with an invalid method name. In this case the server crashes with the following stack trace:

TypeError: Cannot read property 'toLowerCase' of null
    at Route._handles_method (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\route.js:63:21)
    at next (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\index.js:244:30)
    at expressInit (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\middleware\init.js:40:5)
    at Layer.handle [as handle_request] (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\layer.js:95:5)
    at trim_prefix (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\index.js:317:13)
    at D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\index.js:284:7
    at Function.process_params (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\index.js:335:12)
    at next (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\index.js:275:10)
    at query (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\middleware\query.js:45:5)
    at Layer.handle [as handle_request] (D:\Projects\IdeaMixerNodeJS\node_modules\express\lib\router\layer.js:95:5)

If you change if(false) to if(true) the server does not crash and curl returns "empty reply from server". So really don't know if it is a spdy problem or something that comes form the spdy interaction with express. Please advise. Thanks!

[crystalfp]

Well, if I change spdy to https the server does not crash anymore (that is, I changed the require and the spdy invocation).

[yindia]

@crystalfp Hey, I am trying to reproduce the error but i am getting
curl: (52) Empty reply from server without server crashing when i am using curl -k -X INVALID https://localhost:9999/api/kb and the get request working fine so can you provide me your env details

[crystalfp]

I can reproduce it. These are the module versions from my package.json:

  "dependencies": {
    "body-parser": "^1.18.3",
    "commander": "^2.19.0",
    "compression": "^1.7.3",
    "express": "^4.16.4",
    "iconv-lite": "^0.4.24",
    "serve-favicon": "^2.5.0",
    "spdy": "^4.0.0",
    "xml-js": "^1.6.8",
    "yamljs": "^0.3.0"
  },

node -v
v10.13.0

C:/apps/curl/bin/curl.exe --version
curl 7.60.0 (x86_64-pc-win32) libcurl/7.60.0 OpenSSL/1.1.0h (WinSSL) zlib/1.2.11 brotli/1.0.4 WinIDN libssh2/1.8.0 nghttp2/1.32.0
Release-Date: 2018-05-16
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile SSPI Kerberos SPNEGO NTLM SSL libz brotli TLS-SRP HTTP2 HTTPS-proxy MultiSSL

Machine is Windows 10 64 bits

Thanks for your help!
mario

[crystalfp]

I can reproduce also on node version 10.15.0

[yindia]

Check #333 spdy crash with node 10.* . You can follow that thread for future update

[yindia]

So it's a duplicate issue. We can close it. @crystalfp do you want to add something

[crystalfp]

Seems nodejs + http2 is at a turning point. Seems better for me to wait for express 5.0 and get rid of spdy altogether. In the mean time, thanks for your clarifications.
mario

[yindia]

👍