h2non/rocky

Bug in Middleware Flow (only for HEAD Requests?)

alixaxel opened this issue · 2 comments

I believe I've found a bug in the middleware execution flow.

Consider the following simplified example:

const rocky = require('rocky');
const express = require('express');

let snafu = (port) => {
  let app = express();
  let proxy = rocky();
  let methods = ['head', 'get'];

  if (port !== 1337) {
    methods.reverse();
  }

  for (let route of ['/foo']) {
    for (let method of methods) {
      let middlewares = [];

      if (method === 'get') {
        middlewares.push((request, response, next) => {
          console.log(`GET on ${port}.`); return next();
        });
      } else if (method === 'head') {
        middlewares.push((request, response, next) => {
          console.log(`HEAD on ${port}.`); return next();
        });
      }

      proxy[method](route, middlewares).forward('http://localhost:3001');
    }
  }

  app.use(proxy.middleware());
  app.listen(port, () => {
    console.log(`Listening on port ${port}...`);
  });
};

snafu(1337);
snafu(1338); // --> buggy!

The above function will register two middlewares, one for GET and another one for HEAD requests.

The function is called twice, if the port is 1337 the order of the routes will be:

  1. HEAD /foo
  2. GET /foo

Otherwise (if the port is 1338), the order in which the routes will be defined is:

  1. GET /foo
  2. HEAD /foo

And this is the output of the 4 different cURL calls:

  • curl -I "http://localhost:1337/foo" = HEAD on 1337
  • curl -X GET "http://localhost:1337/foo" = GET on 1337
  • curl -I "http://localhost:1338/foo" = GET on 1338 wrong!
  • curl -X GET "http://localhost:1338/foo" = GET on 1338

Curiously enough, if I change the HEAD to a POST I get the expected behavior in all cases.

Is there a reason for this to happen?

rocky relies on router package for HTTP traffic routing.

Please, pass the middleware as variadic parameters using spread syntax instead, such as:

proxy[method](route, ...middlewares).forward('http://localhost:3001');

See supported route interface:
https://github.com/pillarjs/router#routermethodpath-middleware-handler

@tomas-fp Thanks for replying. Actually router package accepts middleware in either format, see here:

var callbacks = flatten(slice.call(arguments))

I originally wrote it with spread operator but the behavior persists.

However, you pointed me in the right direction:

  // append automatic head
  if (this.methods.get && !this.methods.head) {
    methods.push('head')
  }

So this is the behavior that makes the HEAD middleware get ignored if it's defined after the GET.

@alfonsodev