diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index 7fe5d9d5dbdc..bf705bbdbbf4 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -19,6 +19,8 @@ import {Request, Response} from './types'; import {RestBindings} from './keys'; import {RequestContext} from './request-context'; +import {PathParams} from 'express-serve-static-core'; +import {ServeStaticOptions} from 'serve-static'; export class HttpHandler { protected _apiDefinitions: SchemasObject; @@ -48,6 +50,14 @@ export class HttpHandler { this._apiDefinitions = Object.assign({}, this._apiDefinitions, defs); } + registerStaticAssets( + path: PathParams, + rootDir: string, + options?: ServeStaticOptions, + ) { + this._routes.registerStaticAssets(path, rootDir, options); + } + getApiDefinitions() { return this._apiDefinitions; } diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 12d835f3b950..3e698daad76d 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -14,11 +14,12 @@ import { } from '@loopback/openapi-v3-types'; import {AssertionError} from 'assert'; import * as cors from 'cors'; +import * as debugFactory from 'debug'; import * as express from 'express'; import {PathParams} from 'express-serve-static-core'; import {IncomingMessage, ServerResponse} from 'http'; +import {ServerOptions} from 'https'; import {safeDump} from 'js-yaml'; -import * as pathToRegExp from 'path-to-regexp'; import {ServeStaticOptions} from 'serve-static'; import {HttpHandler} from './http-handler'; import {RestBindings} from './keys'; @@ -34,7 +35,6 @@ import { RouteEntry, RoutingTable, } from './router/routing-table'; - import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence'; import { FindRoute, @@ -45,9 +45,8 @@ import { Response, Send, } from './types'; -import {ServerOptions} from 'https'; -const debug = require('debug')('loopback:rest:server'); +const debug = debugFactory('loopback:rest:server'); export type HttpRequestListener = ( req: IncomingMessage, @@ -127,7 +126,6 @@ export class RestServer extends Context implements Server, HttpServerLike { protected _httpServer: HttpServer | undefined; protected _expressApp: express.Application; - protected _routerForStaticAssets: express.Router; get listening(): boolean { return this._httpServer ? this._httpServer.listening : false; @@ -217,9 +215,6 @@ export class RestServer extends Context implements Server, HttpServerLike { }; this._expressApp.use(cors(corsOptions)); - // Place the assets router here before controllers - this._setupRouterForStaticAssets(); - // Set up endpoints for OpenAPI spec/ui this._setupOpenApiSpecEndpoints(); @@ -236,17 +231,6 @@ export class RestServer extends Context implements Server, HttpServerLike { ); } - /** - * Set up an express router for all static assets so that middleware for - * all directories are invoked at the same phase - */ - protected _setupRouterForStaticAssets() { - if (!this._routerForStaticAssets) { - this._routerForStaticAssets = express.Router(); - this._expressApp.use(this._routerForStaticAssets); - } - } - /** * Mount /openapi.json, /openapi.yaml for specs and /swagger-ui, /explorer * to redirect to externally hosted API explorer @@ -626,18 +610,11 @@ export class RestServer extends Context implements Server, HttpServerLike { * See https://expressjs.com/en/4x/api.html#express.static * @param path The path(s) to serve the asset. * See examples at https://expressjs.com/en/4x/api.html#path-examples - * To avoid performance penalty, `/` is not allowed for now. * @param rootDir The root directory from which to serve static assets * @param options Options for serve-static */ static(path: PathParams, rootDir: string, options?: ServeStaticOptions) { - const re = pathToRegExp(path, [], {end: false}); - if (re.test('/')) { - throw new Error( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - } - this._routerForStaticAssets.use(path, express.static(rootDir, options)); + this.httpHandler.registerStaticAssets(path, rootDir, options); } /** diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 59dbbcc6c4c3..51ca3aa01b31 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -27,6 +27,8 @@ import { OperationRetval, } from '../types'; +import * as express from 'express'; + import {ControllerSpec} from '@loopback/openapi-v3'; import * as assert from 'assert'; @@ -35,6 +37,9 @@ const debug = require('debug')('loopback:rest:routing-table'); import {CoreBindings} from '@loopback/core'; import {validateApiPath} from './openapi-path'; import {TrieRouter} from './trie-router'; +import {RequestContext} from '../request-context'; +import {ServeStaticOptions} from 'serve-static'; +import {PathParams} from 'express-serve-static-core'; /** * A controller instance with open properties/methods @@ -84,6 +89,19 @@ export interface RestRouter { export class RoutingTable { constructor(private readonly _router: RestRouter = new TrieRouter()) {} + private _staticAssetsRoute: StaticAssetsRoute; + + registerStaticAssets( + path: PathParams, + rootDir: string, + options?: ServeStaticOptions, + ) { + if (!this._staticAssetsRoute) { + this._staticAssetsRoute = new StaticAssetsRoute(); + } + this._staticAssetsRoute.registerAssets(path, rootDir, options); + } + /** * Register a controller as the route * @param spec @@ -149,6 +167,7 @@ export class RoutingTable { } validateApiPath(route.path); + this._router.add(route); } @@ -181,6 +200,17 @@ export class RoutingTable { return found; } + // this._staticAssetsRoute will be set only if app.static() was called + if (this._staticAssetsRoute) { + debug( + 'No API route found for %s %s, trying to find a static asset', + request.method, + request.path, + ); + + return this._staticAssetsRoute; + } + debug('No route found for %s %s', request.method, request.path); throw new HttpErrors.NotFound( `Endpoint "${request.method} ${request.path}" not found.`, @@ -271,6 +301,88 @@ export abstract class BaseRoute implements RouteEntry { } } +export class StaticAssetsRoute implements RouteEntry, ResolvedRoute { + // ResolvedRoute API + readonly pathParams: PathParameterValues = []; + readonly schemas: SchemasObject = {}; + + // RouteEntry implementation + readonly verb: string = 'get'; + readonly path: string = '/*'; + readonly spec: OperationObject = { + description: 'LoopBack static assets route', + 'x-visibility': 'undocumented', + responses: {}, + }; + + private readonly _expressRouter: express.Router = express.Router(); + + public registerAssets( + path: PathParams, + rootDir: string, + options?: ServeStaticOptions, + ) { + this._expressRouter.use(path, express.static(rootDir, options)); + } + + updateBindings(requestContext: Context): void { + // no-op + } + + async invokeHandler( + {request, response}: RequestContext, + args: OperationArgs, + ): Promise { + const handled = await executeRequestHandler( + this._expressRouter, + request, + response, + ); + + if (!handled) { + // Express router called next, which means no route was matched + throw new HttpErrors.NotFound( + `Endpoint "${request.method} ${request.path}" not found.`, + ); + } + } + + describe(): string { + return 'final route to handle static assets'; + } +} + +/** + * Execute an Express-style callback-based request handler. + * + * @param handler + * @param request + * @param response + * @returns A promise resolved to: + * - `true` when the request was handled + * - `false` when the handler called `next()` to proceed to the next + * handler (middleware) in the chain. + */ +function executeRequestHandler( + handler: express.RequestHandler, + request: Request, + response: express.Response, +): Promise { + return new Promise((resolve, reject) => { + const onceFinished = () => resolve(true); + response.once('finish', onceFinished); + handler(request, response, (err: Error) => { + response.removeListener('finish', onceFinished); + if (err) { + reject(err); + } else { + // Express router called next, which means no route was matched + resolve(false); + } + }); + }); +} + export function createResolvedRoute( route: RouteEntry, pathParams: PathParameterValues, diff --git a/packages/rest/test/integration/rest.application.integration.ts b/packages/rest/test/integration/rest.application.integration.ts new file mode 100644 index 000000000000..50577c5e4674 --- /dev/null +++ b/packages/rest/test/integration/rest.application.integration.ts @@ -0,0 +1,48 @@ +// Copyright IBM Corp. 2017,2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {createRestAppClient, Client} from '@loopback/testlab'; +import {RestApplication} from '../..'; +import * as path from 'path'; +import * as fs from 'fs'; + +const FIXTURES = path.resolve(__dirname, '../../../fixtures'); + +describe('RestApplication (integration)', () => { + let restApp: RestApplication; + let client: Client; + + beforeEach(givenAnApplication); + beforeEach(givenClient); + afterEach(async () => { + await restApp.stop(); + }); + + it('serves static assets', async () => { + const root = FIXTURES; + const content = fs + .readFileSync(path.join(root, 'index.html')) + .toString('utf-8'); + await client + .get('/index.html') + .expect(200) + .expect(content); + }); + + it('returns 404 if asset is not found', async () => { + await client.get('/404.html').expect(404); + }); + + function givenAnApplication() { + const root = FIXTURES; + restApp = new RestApplication({rest: {port: 0, host: '127.0.0.1'}}); + restApp.static('/', root); + } + + async function givenClient() { + await restApp.start(); + return (client = createRestAppClient(restApp)); + } +}); diff --git a/packages/rest/test/integration/rest.server.integration.ts b/packages/rest/test/integration/rest.server.integration.ts index 99a6df3c586a..8933871f56a5 100644 --- a/packages/rest/test/integration/rest.server.integration.ts +++ b/packages/rest/test/integration/rest.server.integration.ts @@ -80,7 +80,7 @@ describe('RestServer (integration)', () => { .expect(500); }); - it('does not allow static assets to be mounted at /', async () => { + it('allows static assets to be mounted at /', async () => { const root = FIXTURES; const server = await givenAServer({ rest: { @@ -88,29 +88,12 @@ describe('RestServer (integration)', () => { }, }); - expect(() => server.static('/', root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - - expect(() => server.static('', root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - - expect(() => server.static(['/'], root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - - expect(() => server.static(['/html', ''], root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - - expect(() => server.static(/.*/, root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); - - expect(() => server.static('/(.*)', root)).to.throw( - 'Static assets cannot be mount to "/" to avoid performance penalty.', - ); + expect(() => server.static('/', root)).to.not.throw(); + expect(() => server.static('', root)).to.not.throw(); + expect(() => server.static(['/'], root)).to.not.throw(); + expect(() => server.static(['/html', ''], root)).to.not.throw(); + expect(() => server.static(/.*/, root)).to.not.throw(); + expect(() => server.static('/(.*)', root)).to.not.throw(); }); it('allows static assets via api', async () => { @@ -164,7 +147,7 @@ describe('RestServer (integration)', () => { .expect(200, 'Hello'); }); - it('serve static assets if matches before other routes', async () => { + it('gives precedence to API routes over static assets', async () => { const root = FIXTURES; const server = await givenAServer({ rest: { @@ -174,12 +157,9 @@ describe('RestServer (integration)', () => { server.static('/html', root); server.handler(dummyRequestHandler); - const content = fs - .readFileSync(path.join(root, 'index.html')) - .toString('utf-8'); await createClientForHandler(server.requestHandler) .get('/html/index.html') - .expect(200, content); + .expect(200, 'Hello'); }); it('allows cors', async () => {