Skip to content

Commit

Permalink
use resolve-path
Browse files Browse the repository at this point in the history 
closes #9
  • Loading branch information
haoxin committed Feb 6, 2015
1 parent 78be346 commit 74817ba
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 59 deletions.
5 changes: 3 additions & 2 deletions Readme.md
View file
Expand Up @@ -25,8 +25,9 @@ $ npm install koa-send

## Root path

Note that when `root` is _not_ used you __MUST__ provide an _absolute_
path, and this path must not contain "..", protecting developers from
Note that `root` is required, defaults to `''` and will be resolved,
removing the leading `/` to make the path relative and this
path must not contain "..", protecting developers from
concatenating user input. If you plan on serving files based on
user input supply a `root` directory from which to serve from.

Expand Down
30 changes: 3 additions & 27 deletions index.js
View file
Expand Up @@ -3,14 +3,14 @@
*/

var debug = require('debug')('koa-send');
var resolvePath = require('resolve-path');
var assert = require('assert');
var path = require('path');
var normalize = path.normalize;
var basename = path.basename;
var extname = path.extname;
var resolve = path.resolve;
var fs = require('mz/fs');
var join = path.join;

/**
* Expose `send()`.
Expand All @@ -37,6 +37,7 @@ function send(ctx, path, opts) {
// options
debug('send "%s" %j', path, opts);
var root = opts.root ? normalize(resolve(opts.root)) : '';
path = path[0] == '/' ? path.slice(1) : path;
var index = opts.index;
var maxage = opts.maxage || opts.maxAge || 0;
var hidden = opts.hidden || false;
Expand All @@ -51,21 +52,10 @@ function send(ctx, path, opts) {

if (-1 == path) return ctx.throw('failed to decode', 400);

// null byte(s)
if (~path.indexOf('\0')) return ctx.throw('null bytes', 400);

// index file support
if (index && trailingSlash) path += index;

// malicious path
if (!root && !isAbsolute(path)) return ctx.throw('relative paths require the .root option', 500);
if (!root && ~path.indexOf('..')) return ctx.throw('malicious path', 400);

// relative to root
path = normalize(join(root, path));

// out of bounds
if (root && 0 !== path.indexOf(root)) return ctx.throw('malicious path', 400);
path = resolvePath(root, path);

// hidden file support, ignore
if (!hidden && leadingDot(path)) return;
Expand Down Expand Up @@ -126,17 +116,3 @@ function decode(path) {
return -1;
}
}

/**
* Check if `path` looks absolute.
*
* @param {String} path
* @return {Boolean}
* @api private
*/

function isAbsolute(path){
if ('/' == path[0]) return true;
if (':' == path[1] && '\\' == path[2]) return true;
if ('\\\\' == path.substring(0, 2)) return true; // Microsoft Azure absolute path
}
3 changes: 2 additions & 1 deletion package.json
View file
Expand Up @@ -22,7 +22,8 @@
"license": "MIT",
"dependencies": {
"debug": "*",
"mz": "^1.0.1"
"mz": "^1.0.1",
"resolve-path": "^1.2.1"
},
"scripts": {
"test": "mocha --harmony-generators --require should --reporter spec",
Expand Down
74 changes: 45 additions & 29 deletions test/index.js
View file
Expand Up @@ -7,7 +7,7 @@ var assert = require('assert');
describe('send(ctx, file)', function(){
describe('with no .root', function(){
describe('when the path is absolute', function(){
it('should serve the file', function(done){
it('should 404', function(done){
var app = koa();

app.use(function *(){
Expand All @@ -16,13 +16,12 @@ describe('send(ctx, file)', function(){

request(app.listen())
.get('/')
.expect(200)
.expect('world', done);
.expect(404, done);
})
})

describe('when the path is relative', function(){
it('should 500', function(done){
it('should 200', function(done){
var app = koa();

app.use(function *(){
Expand All @@ -31,21 +30,22 @@ describe('send(ctx, file)', function(){

request(app.listen())
.get('/')
.expect(500, done);
.expect(200)
.expect('world', done);
})
})

describe('when the path contains ..', function(){
it('should 400', function(done){
it('should 403', function(done){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/../fixtures/hello.txt');
yield send(this, '/../fixtures/hello.txt');
});

request(app.listen())
.get('/')
.expect(400, done);
.expect(403, done);
})
})
})
Expand Down Expand Up @@ -98,7 +98,7 @@ describe('send(ctx, file)', function(){
})

describe('when the path resolves above the root', function(){
it('should 400', function(done){
it('should 403', function(done){
var app = koa();

app.use(function *(){
Expand All @@ -108,12 +108,12 @@ describe('send(ctx, file)', function(){

request(app.listen())
.get('/')
.expect(400, done);
.expect(403, done);
})
})

describe('when the path resolves within root', function(){
it('should 400', function(done){
it('should 403', function(done){
var app = koa();

app.use(function *(){
Expand All @@ -123,8 +123,7 @@ describe('send(ctx, file)', function(){

request(app.listen())
.get('/')
.expect(200)
.expect('html index', done);
.expect(403, done);
})
})
})
Expand Down Expand Up @@ -152,7 +151,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/test');
yield send(this, '/test');
});

request(app.listen())
Expand All @@ -164,7 +163,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
var sent = yield send(this, __dirname + '/test');
var sent = yield send(this, '/test');
assert.equal(sent, undefined);
});

Expand All @@ -179,7 +178,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures');
yield send(this, '/test/fixtures');
});

request(app.listen())
Expand All @@ -188,15 +187,29 @@ describe('send(ctx, file)', function(){
})
})

describe('when path is malformed', function(){
it('should 400', function(done){
var app = koa();

app.use(function *(){
yield send(this, '/%');
});

request(app.listen())
.get('/')
.expect(400, done);
})
})

describe('when path is a file', function(){

it('should return the path', function(done){
var app = koa();

app.use(function *(){
var p = __dirname + '/fixtures/user.json';
var p = '/test/fixtures/user.json';
var sent = yield send(this, p);
assert.equal(sent, p);
assert.equal(sent, __dirname + '/fixtures/user.json');
});

request(app.listen())
Expand All @@ -209,7 +222,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/gzip.json');
yield send(this, '/test/fixtures/gzip.json');
});

request(app.listen())
Expand All @@ -219,11 +232,12 @@ describe('send(ctx, file)', function(){
.expect('{ "name": "tobi" }')
.expect(200, done);
})

it('should return .gz path (gzip option defaults to true)', function(done){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/gzip.json');
yield send(this, '/test/fixtures/gzip.json');
});

request(app.listen())
Expand All @@ -233,11 +247,12 @@ describe('send(ctx, file)', function(){
.expect('{ "name": "tobi" }')
.expect(200, done);
})

it('should return .gz path when gzip option is turned on', function(done){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/gzip.json', {gzip: true});
yield send(this, '/test/fixtures/gzip.json', { gzip: true });
});

request(app.listen())
Expand All @@ -247,11 +262,12 @@ describe('send(ctx, file)', function(){
.expect('{ "name": "tobi" }')
.expect(200, done);
})

it('should not return .gz path when gzip option is false', function(done){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/gzip.json', {gzip: false});
yield send(this, '/test/fixtures/gzip.json', { gzip: false });
});

request(app.listen())
Expand All @@ -268,9 +284,9 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
var p = __dirname + '/fixtures/user.json';
var p = '/test/fixtures/user.json';
var sent = yield send(this, p, { maxage: 5000 });
assert.equal(sent, p);
assert.equal(sent, __dirname + '/fixtures/user.json');
});

request(app.listen())
Expand All @@ -283,9 +299,9 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
var p = __dirname + '/fixtures/user.json';
var p = '/test/fixtures/user.json';
var sent = yield send(this, p, { maxage: 1234 });
assert.equal(sent, p);
assert.equal(sent, __dirname + '/fixtures/user.json');
});

request(app.listen())
Expand All @@ -300,7 +316,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/user.json');
yield send(this, '/test/fixtures/user.json');
});

request(app.listen())
Expand All @@ -313,7 +329,7 @@ describe('send(ctx, file)', function(){
var app = koa();

app.use(function *(){
yield send(this, __dirname + '/fixtures/user.json');
yield send(this, '/test/fixtures/user.json');
});

request(app.listen())
Expand All @@ -327,7 +343,7 @@ describe('send(ctx, file)', function(){
var stream

app.use(function *(){
yield send(this, __dirname + '/fixtures/user.json');
yield send(this, '/test/fixtures/user.json');
stream = this.body;
this.socket.emit('error', new Error('boom'));
})
Expand Down

0 comments on commit 74817ba

Please sign in to comment.