The connection must not be reset for "HTTP2-Settings" request

[mykola-mokhnach]

We are using express.js 4.19.2 as HTTP server in Appium and we can observe an unexpected behaviour when Java HTTP client or any other client with HTTP/2 support tries to connect to it. Such clients usually try to send a request to upgrade the connection to HTTP/2, which contains the Connection: Upgrade, HTTP2-Settings. When that happens the server does not respond according to the standard, which makes JdkHttpClient to fail with java.io.IOException: HTTP/1.1 header parser received no bytes. Enforcing the client to not send this header (e.g. HTTP/1.1 mode) does not reproduce the above exception. See SeleniumHQ/selenium#13913 for more details.

Unfortunately I was not able to understand what exactly makes java client so unhappy about the current server response, but it would be nice to have at least some server-side workaround for this issue.

[wesleytodd]

This is not a bug in express. Express does not currently support http2, so making an such a request to an express server is expected not to work. You might be able to check for the connection header in your app code and reply with a 4xx error to tell the client this is not supported. I have never tried that, so maybe Node.js will handle that in an unexpected way I am not sure. But either way I am going to close this because it is not an express bug.

[mykola-mokhnach]

Thank you for your response @wesleytodd

After deeper investigation I have figured out the problem lies in the server's 'upgrade' event listener. Basically, the request to upgrade to HTTP2 is similar to the websocket upgrade one so it gets forwarded to the websocket handler which raises the above behaviour. Maybe you could kindly advice what shall I put into the event handler to tell express to not handle the request as websocket upgrade and simply fallback to a "normal" http?

My event listener looks like

    this.on('upgrade', (request, socket, head) => {
      if (_.toLower(request.headers.upgrade) !== 'websocket') {
        // What should I put here to fallback to a "normal" http handler?
        return;
      }
      let currentPathname;
      try {
        currentPathname = new URL(request.url ?? '').pathname;
      } catch {
        currentPathname = request.url ?? '';
      }
      for (const [pathname, wsServer] of _.toPairs(this.webSocketsMapping)) {
        if (pathToRegexp(pathname).test(currentPathname)) {
          wsServer.handleUpgrade(request, socket, head, (ws) => {
            wsServer.emit('connection', ws, request);
          });
          return;
        }
      }
      socket.destroy();
    });
  }

[mykola-mokhnach]

For now I have decided to change the upgrade requests handling strategy and removed the 'upgrade' even listener completely. Instead, I've added another middleware. The idea there is to check the actual header value and decide what to do next based on whether this is a webcoket upgrade request or not:

export function handleUpgrade(webSocketsMapping) {
  return (req, res, next) => {
    if (!req.headers?.upgrade || _.toLower(req.headers.upgrade) !== 'websocket') {
      return next();
    }
    let currentPathname;
    try {
      currentPathname = new URL(req.url ?? '').pathname;
    } catch {
      currentPathname = req.url ?? '';
    }
    for (const [pathname, wsServer] of _.toPairs(webSocketsMapping)) {
      if (pathToRegexp(pathname).test(currentPathname)) {
        return wsServer.handleUpgrade(req, req.socket, Buffer.from(''), (ws) => {
          wsServer.emit('connection', ws, req);
        });
      }
    }
    log.info(`Did not match the websocket upgrade request at ${currentPathname} to any known route`);
    next();
  };
}

The whole PR: appium/appium#20142