fix(sendFile): surface resolved path in "No such file" error; clarify no auto-versioning (#257)

README.md

@@ -878,6 +878,8 @@ Path parameters act as wildcards that capture the value into the `params` object
 
 A path can contain as many parameters as you want. E.g. `/users/:param1/:param2/:param3`.
 
+Lambda API performs **no automatic version handling or stripping**. Version-like segments such as `v1` are treated as ordinary path values, so a route like `/:v/schema.json` matches `/v1/schema.json` and captures `req.params.v === 'v1'`. If you need versioned route groups, set them up explicitly with a `base` path or [Route Prefixing](#route-prefixing). Note that setting `base` to a version (e.g. `base: 'v1'`) makes that literal segment collide with a leading path parameter — prefer a non-version `base` like `/api` and use `register({ prefix: '/v1' })` for versioning. Remember too that `res.sendFile(path, { root })` resolves to `root + path` by plain string concatenation (not `path.join`), so passing `req.path` (which includes any `base`/prefix segments) into `sendFile` looks for that full path on disk. Because it's literal concatenation, make sure `root` ends with a trailing slash (or `path` begins with one) — otherwise `root: './static'` + `'file.txt'` resolves to `./staticfile.txt` rather than `./static/file.txt`.
+
 ## Wildcard Routes
 
 Wildcard routes are supported for matching arbitrary paths. Wildcards only work at the _end of a route definition_ such as `/*` or `/users/*`. Wildcards within a path, e.g. `/users/*/posts` are not supported. Wildcard routes do support parameters, however, so `/users/:id/*` would capture the `:id` parameter in your wildcard handler.

__tests__/download.unit.js

@@ -155,7 +155,7 @@ describe('Download Tests:', function() {
   it('Missing file', async function() {
     let _event = Object.assign({},event,{ path: '/download' })
     let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
-    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'], 'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file"}', isBase64Encoded: false })
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'], 'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: ./test-missing.txt"}', isBase64Encoded: false })
   }) // end it
 
   it('Missing file with custom catch', async function() {

__tests__/sendFile.unit.js

@@ -37,6 +37,18 @@ api.get('/sendfile/root', function(req,res) {
   res.sendFile('test.txt', { root: './__tests__/' })
 })
 
+// Missing file under an absolute `root` -- the root prefix must be stripped
+// from the client-visible error so internal server paths aren't disclosed.
+api.get('/sendfile/root-missing', function(req,res) {
+  res.sendFile('missing.txt', { root: '/var/task/private/' })
+})
+
+// Missing file passed as an absolute path (no root) -- reduced to its basename
+// in the client-visible error for the same reason.
+api.get('/sendfile/abs-missing', function(req,res) {
+  res.sendFile('/var/task/private/secret.txt')
+})
+
 api.get('/sendfile/err', function(req,res) {
   res.sendFile('./test-missing.txt', err => {
     if (err) {
@@ -201,7 +213,19 @@ describe('SendFile Tests:', function() {
   it('Missing file', async function() {
     let _event = Object.assign({},event,{ path: '/sendfile' })
     let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
-    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file"}', isBase64Encoded: false })
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: ./test-missing.txt"}', isBase64Encoded: false })
+  }) // end it
+
+  it('Missing file under absolute root strips the root prefix', async function() {
+    let _event = Object.assign({},event,{ path: '/sendfile/root-missing' })
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: missing.txt"}', isBase64Encoded: false })
+  }) // end it
+
+  it('Missing file with absolute path is reduced to basename', async function() {
+    let _event = Object.assign({},event,{ path: '/sendfile/abs-missing' })
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'],'x-error': ['true'] }, statusCode: 500, body: '{"error":"No such file: secret.txt"}', isBase64Encoded: false })
   }) // end it
 
   it('Missing file with custom catch', async function() {

__tests__/versioning.unit.js

@@ -0,0 +1,100 @@
+'use strict';
+
+// Regression coverage for GitHub issue #257 ("Is it possible to disable versioning?").
+//
+// lambda-api does NOT perform any automatic version detection or stripping. A
+// version-like path segment (e.g. `v1`) is matched literally and exposed on
+// `req.params` just like any other parameter value. These tests lock that in so
+// the behavior can never silently regress.
+
+// Init API instance (no base -- matching the reporter's setup)
+const api = require('../index')();
+
+// Sample events for the other supported interfaces. Route matching must not
+// diverge across API Gateway v1 (`path`), API Gateway v2 (`rawPath`) and ALB
+// (`path` + `requestContext.elb`), so the same version-like segment is asserted
+// against each shape below. Deep-cloned per test so overrides don't leak.
+const sampleV2 = require('./sample-event-apigateway-v2.json');
+const sampleAlb = require('./sample-event-alb1.json');
+const clone = (obj) => JSON.parse(JSON.stringify(obj));
+
+// NOTE: Set test to true
+api._test = true;
+
+let event = {
+  httpMethod: 'get',
+  path: '/',
+  body: {},
+  multiValueHeaders: {
+    'Content-Type': ['application/json']
+  }
+}
+
+/******************************************************************************/
+/***  DEFINE TEST ROUTES                                                    ***/
+/******************************************************************************/
+api.get('/:v/schema.json', function(req,res) {
+  res.status(200).json({ method: 'get', status: 'ok', v: req.params.v })
+})
+
+/******************************************************************************/
+/***  BEGIN TESTS                                                           ***/
+/******************************************************************************/
+
+describe('Versioning (issue #257) Tests:', function() {
+
+  it('Version-like segment is matched literally: /v1/schema.json', async function() {
+    let _event = Object.assign({},event,{ path: '/v1/schema.json' })
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 200, body: '{"method":"get","status":"ok","v":"v1"}', isBase64Encoded: false })
+  }) // end it
+
+  it('API Gateway v2 (rawPath) matches the version-like segment literally: /v1/schema.json', async function() {
+    let _event = clone(sampleV2)
+    _event.rawPath = '/v1/schema.json'
+    _event.requestContext.http.method = 'GET'
+    _event.requestContext.http.path = '/v1/schema.json'
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result.statusCode).toBe(200)
+    expect(JSON.parse(result.body).v).toBe('v1')
+  }) // end it
+
+  it('ALB event (path) matches the version-like segment literally: /v1/schema.json', async function() {
+    let _event = clone(sampleAlb)
+    _event.httpMethod = 'GET'
+    _event.path = '/v1/schema.json'
+    _event.isBase64Encoded = false
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result.statusCode).toBe(200)
+    expect(JSON.parse(result.body).v).toBe('v1')
+  }) // end it
+
+  it('Non-version first segment is matched the same way: /abc/schema.json', async function() {
+    let _event = Object.assign({},event,{ path: '/abc/schema.json' })
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 200, body: '{"method":"get","status":"ok","v":"abc"}', isBase64Encoded: false })
+  }) // end it
+
+  it('No version segment is consumed -- extra segment still 404s: /v1/x/schema.json', async function() {
+    let _event = Object.assign({},event,{ path: '/v1/x/schema.json' })
+    let result = await new Promise(r => api.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 404, body: '{"error":"Route not found"}', isBase64Encoded: false })
+  }) // end it
+
+  it("Setting base to a version collides with a leading :param (documented caveat)", async function() {
+    // With `base: 'v1'`, the literal `v1` base segment consumes the first path
+    // segment, so `/v1/schema.json` matches `v1` + `:v` (schema.json) and never
+    // reaches the handler -- yielding a 405. Prefer a base like `/api` over a
+    // version, or use `register({ prefix })` for versioned route groups.
+    const basedApi = require('../index')({ base: 'v1' })
+    basedApi._test = true;
+    basedApi.get('/:v/schema.json', function(req,res) {
+      res.status(200).json({ v: req.params.v })
+    })
+
+    let _event = Object.assign({},event,{ path: '/v1/schema.json' })
+    let result = await new Promise(r => basedApi.run(_event,{},(e,res) => { r(res) }))
+    expect(result).toEqual({ multiValueHeaders: { 'content-type': ['application/json'] }, statusCode: 405, body: '{"error":"Method not allowed"}', isBase64Encoded: false })
+  }) // end it
+
+}) // end VERSIONING tests

src/lib/response.js

@@ -397,7 +397,23 @@ class RESPONSE {
 
       // If missing file
       if (e.code === 'ENOENT') {
-        this.error(new FileError('No such file', e));
+        // Surface the missing path so the cause is self-evident, but avoid
+        // leaking absolute server paths (e.g. /var/task/...) in the
+        // client-visible error body: strip any configured `root` prefix and
+        // reduce a still-absolute path to its basename. The full `e.path` is
+        // preserved on the FileError for server-side logging.
+        let display = e.path;
+        if (display) {
+          if (opts.root && display.startsWith(opts.root)) {
+            display = display.slice(opts.root.length);
+          }
+          if (path.isAbsolute(display)) {
+            display = path.basename(display);
+          }
+        }
+        this.error(
+          new FileError(`No such file${display ? `: ${display}` : ''}`, e)
+        );
       } else {
         this.error(e); // Throw error if not done in callback
       }