restify / plugins Goto Github PK

Exposed via restify/node-restify#928

@micahr, @yunong, @mmouterde I have a question about the change from #39. Recently I updated the 'queryParser' plugin in restify 4.x to also pass through options to qs.parse in restify/node-restify#1209 However, in the change to restify I (a) limited the set of options that were passed through and (b) explicitly listed and validated the types of the passed through options.

The set of qs.parse options passed through are:

var EXPOSED_QS_OPTIONS = {
    allowDots: assert.optionalBool,
    arrayLimit: assert.optionalNumber,
    depth: assert.optionalNumber,
    parameterLimit: assert.optionalNumber,
    parseArrays: assert.optionalBool,
    plainObjects: assert.optionalBool,
    strictNullHandling: assert.optionalBool

    /*
     * Exclusions (`qs.parse` options that restify does NOT expose):
     * - `allowPrototypes`: It is strongly suggested against in qs docs.
     * - `decoder`
     * - `delimiter`: For query string parsing we shouldn't support anything
     *   but the default '&'.
     */
};

That shows reasons (if any, other than tending to not add features without need) for excluding a few of the options.

Thoughts on whether we want to do the same for restify-plugins' queryParser? I.e. should I submit a PR to restify-plugins to explicit list, assert type on, and limit the set of qs.parse options?

Happening on Firefox 43.0 for me today.

// This path points at the application root. The folder is called "docs"
const docPath = path.normalize(path.join(path.dirname(__filename), '../'))

// We then restrict access to only the documentation directory
server.get(/\/docs\//, restify.serveStatic({
    directory: docPath,
    default: 'index.html',
    // Cache sends everything as text/plain after first load.
    // I have no idea why. Setting maxAge to zero fixes, but never caches.
    // maxAge: 0,
}))

Can anyone reproduce? First load is all good, but next load everything is raw text and content-types are all messed up.

The restify plugins are finding themselves a new (well, old?) home back in the node-restify repo. As part of this process, we need to properly deprecate this repo.

TODO:

• Migrate plugins to main repo
• Add notice to top of README.md
• Add notice to description on GitHub repo
• npm deprecate
• Create ISSUE_TEMPLATE and PULL_REQUEST_TEMPLATE pointing back to node-restify
• Release 5.x w/ bundled plugins

Is there value in exposing a way to allow users to pass in a map of types to parsers? Or do we continue to encourage users to layer on their own custom parsers in addition to the currently provided one?

Given that there are some conventions, i.e., req.rawBody and req.body that restify "owns" when it parses a body, there's an argument to be made at least for consistency.

Here and here need replace property logMetrics to logBuffer because audit.js have not this option.

After replace and run test, we will see a bug audit.js.

If we write this:

console.log('****************************** before metrics.latency:', metrics.latency);
assert.isAtLeast(metrics.latency, 200);
console.log('****************************** after metrics.latency:', metrics.latency);

here, and run test, we will see before metrics.latency but not after metrics.latency.

It seems that here bug in mocha and restify-plugins.

I want to serve all assets except index.html with a distant expiration so as to use cache-busting selectively. The problem with the below approach is that serveStatic imposes a conflicting Cache-Control header.

server.pre((req, res, next) => {
    const path = url.parse(req.url);
    if (path.pathname === '/' && req.accepts('text/html')) {
        req.url = '/static/index.html';

        // Encourage the client to always download index.html.
        res.header('Expires', new Date(0));
        res.header('Cache-Control',
            'no-store, no-cache, must-revalidate');
    }

    next();
});

server.get(/\/static\/.*/, restify.serveStatic({
    directory: path.resolve('.'),
    maxAge: (10 * 365 * 24 * 60 * 60),
    gzip: true,
}));

In this setup, my response has the following conflicting headers:

Cache-Control:no-store, no-cache, must-revalidate, max-age=0
Cache-Control:public, max-age=315360000

I try to make this more explicit by using the file option. But oddly enough, the below 404s requests to index.html. Probably my fault, but hell if I know why.

server.pre((req, res, next) => {
    const path = url.parse(req.url);
    if (path.pathname === '/' && req.accepts('text/html')) {
        req.url = '/public/index.html';

        // Encourage the client to always download index.html.
        res.header('Expires', new Date(0));
        res.header('Cache-Control',
            'no-store, no-cache, must-revalidate');
    }

    next();
});

const directory = path.resolve('.');

const serveDir = restify.serveStatic({
    directory,
    maxAge: (10 * 365 * 24 * 60 * 60),
    gzip: true
});

const serveIndex = restify.serveStatic({
    directory,
    file: 'index.html',
    maxAge: 0,
    gzip: false
});

server.get(/\/public\/.*/, function(req, res, next) {
    if (req.url === '/public/index.html') {
        serveIndex(req, res, next);
    } else {
        serveDir(req, res, next);
    }
});

The point is that I am rewriting my code around serveStatic's inability to omit max-age (instead opting for a 3600 default), and I wonder why I should. Is it possible to have serveStatic trust me to write my own headers, or is there a better way to go about this?

sanitizePath currently removes duplicate trailing slashes. With support in core now for strict routing in restify/node-restify#1092, this plugin should only remove duplicate slashes in the url.

This lets core make the determination whether or not it supports trailing slashes.

The initiating user is an important part of any audit trail. It is common practice to store the currently active user in req.user, however the auditRequestSerializer does not include the user property, and cannot be externally overridden.

Option A:
Update the auditRequestSerializer to include req.user

Option B:
Update the auditLogger to allow the serializers to be overridden via options (with a fallback to the existing one as a default)

Add options for specifying two headers that combined, equal the absolute time that should be used for expiration.

CORS is finding a new (possibly temporary) home: Tabcorp/restify-cors-middleware#27

The implementation left in this repo has quite a few issues. Lets rip it out in favor of that PR.

Currently the serveStatic plugin is affected by the following Node.js bug: nodejs/node#1834

If a client disconnects too early a file stream (and a file handle) will be leaked permanently, which allows anyone to trivially DoS any server that uses the serveStatic plugin.

Since it doesn't look like the Node.js bug is going to be fixed anytime soon it should be worked around here.

How to reproduce the issue

1. Create a public directory and put a non-empty index.html in it.

2. Run this as a server:

   var restify = require('restify');
   var plugins = require('restify-plugins');
   
   var server = restify.createServer({});
   
   server.use(plugins.acceptParser(server.acceptable));
   server.use(plugins.queryParser());
   server.use(plugins.bodyParser());
   
   server.listen(8091);
   server.get(/.*/, plugins.serveStatic({
     directory: './public'
   }));
   

3. Run this as a client:

   var net = require('net');
   var LEAKY = true;
   
   setInterval( function() {
       var socket = new net.Socket();
       socket.connect({host: '127.0.0.1', port: 8091}, function() {
           var DATA =
               "GET /index.html HTTP/1.1\r\n" +
               "Host: localhost:8091\r\n" +
               "User-Agent: curl/7.48.0\r\n" +
               "Accept: */*\r\n" +
               "\r\n";
   
           socket.write( DATA, 'utf-8', function() {
               if (LEAKY) {
                   socket.end();
               } else {
                   setTimeout( function() {
                       socket.end();
                   }, 10);
               }
           });
       });
   }, 100);
   

4. Run watch -n 0.1 "stat /proc/$server_pid/fd/* | grep File | wc -l"; the number of open file handles will continue to grow indefinitely.

5. If you change LEAKY to false then the issue will stop reproducing.

Versions

restify 4.1.1, restify-plugins 1.0.2

I'd like to be able to set some of the options of the csv parser used internally by csv-parse, especially relax_column_count.

I know that I can override some of the default options with request headers, but not everything is exposed and not everything should be left to the client to decide

(reposted from restify/node-restify#1167)

could we add application/EDIFACT mime type as defined in rfc 1767 to this file https://github.com/restify/plugins/blob/master/lib/plugins/bodyParser.js? afaik, there is one node library that already support this mime type, that is https://www.npmjs.com/package/edi

thanks

We need a new plugin whose role is only to remove duplicate slashes in the URL, now that restify core supports trailing slashes in the URL.

... or enabled only if occurs any errors.

Like most rate limiting it should place the limits in the response header.

https://travis-ci.org/restify/plugins/jobs/202762529

Core has changed, PORT is no longer automatically assigned in some scenarios. Specify a manual PORT number for unit tests.

Hi, I'm wanna contribute with bearer authorization parse but when I use "make test", returns some errors...
Could you iluminate me? :)

The documentation for the pre.context plugin states, "Provide req.set(key, val) and req.get(key) methods for setting and retrieving context to a specific request". But the code in fact maintains the data structure over the lifetime of the app, not the request. Is this actually intentional? It resulted in some very unexpected behavior in my app.

The very simple fix is to move var data = {} to the first line of the context function.

Hi;

I am trying to serve 2 apps with serveStatic plugin.

server.get('/app1', restify.serveStatic({
directory: __dirname + '/shared/.build',
default: 'index.html'
}));

is not working but

server.use('/app1', express.static(__dirname + '/shared/.build'));

works as expected.

Where am I wrong?

In addition, each definition has excerpts from documentation.

Install

npm install @types/restify-plugins --save-dev

Also, you can use new declarations for assert-plus:

npm install @types/assert-plus --save-dev