Skip to content
This repository has been archived by the owner on Mar 16, 2022. It is now read-only.

Bug in Middleware Flow (only for HEAD Requests?) #100

Open
alixaxel opened this issue Nov 24, 2016 · 2 comments
Open

Bug in Middleware Flow (only for HEAD Requests?) #100

alixaxel opened this issue Nov 24, 2016 · 2 comments

Comments

@alixaxel
Copy link

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?

@tomas-fp
Copy link

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

@alixaxel
Copy link
Author

@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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants