Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(http-server): adds http/2 support #4989

Closed
wants to merge 3 commits into from
Closed

feat(http-server): adds http/2 support #4989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad85271
feat(http-server): adds http/2 support
achrinza Mar 29, 2020
ef72b29
feat(testlab): add http2 helpers
achrinza Apr 16, 2020
28b8b5e
test(http-server): add http2 integration tests
achrinza Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion packages/http-server/src/__tests__/integration/http-server.integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import {
expect,
givenHttpServerConfig,
Http2sOptions,
httpGetAsync,
httpsGetAsync,
skipOnTravis,
Expand All @@ -18,7 +19,7 @@ import os from 'os';
import pEvent from 'p-event';
import path from 'path';
import {HttpOptions, HttpServer, HttpsOptions} from '../../';
import {HttpServerOptions} from '../../http-server';
import {Http2Options, HttpServerOptions} from '../../http-server';

describe('HttpServer (integration)', () => {
let server: HttpServer | undefined;
Expand Down Expand Up @@ -294,6 +295,13 @@ describe('HttpServer (integration)', () => {
}).to.throw(/Named pipe test\.pipe does NOT start with/);
});

it('supports HTTP/2 HTTP', async () => {
const http2Server = givenHttp2Server();
await http2Server.start();
const response = await httpGetAsync(http2Server.url);
expect(response.statusCode).to.equal(200);
});

function getAddressFamily(httpServer: HttpServer) {
if (!httpServer || !httpServer.address) return undefined;
if (typeof httpServer.address === 'string') {
Expand Down Expand Up @@ -353,4 +361,25 @@ describe('HttpServer (integration)', () => {
}
return new HttpServer(dummyRequestHandler, options);
}

function givenHttp2Server(): HttpServer {
const options = givenHttpServerConfig<Http2Options>({
protocol: 'http2',
});

return new HttpServer(dummyRequestHandler, options);
}

function givenHttp2sServer({
allowHttp1 = false,
}: {
allowHttp1: boolean;
}): HttpServer {
const options = givenHttpServerConfig<Http2sOptions>({
protocol: 'http2s',
allowHTTP1: allowHttp1,
});

return new HttpServer(dummyRequestHandler, options);
}
});
89 changes: 75 additions & 14 deletions packages/http-server/src/http-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import assert from 'assert';
import http, {IncomingMessage, ServerResponse} from 'http';
import http2 from 'http2';
import https from 'https';
import {AddressInfo, ListenOptions} from 'net';
import os from 'os';
Expand All @@ -19,6 +20,14 @@ export type RequestListener = (
res: ServerResponse,
) => void;

/**
* Request listener function for http2/https2 requests
*/
export type Http2RequestListener = (
req: http2.Http2ServerRequest,
res: http2.Http2ServerResponse,
) => void;

/**
* Base options that are common to http and https servers
*/
Expand Down Expand Up @@ -52,17 +61,37 @@ export interface HttpsOptions extends BaseHttpOptions, https.ServerOptions {
protocol: 'https';
}

/**
* HTTP/2 HTTP server options
*/
export interface Http2Options extends BaseHttpOptions, http2.ServerOptions {
protocol: 'http2';
}

/**
* HTTP/2 HTTPS server options
*/
export interface Https2Options
extends BaseHttpOptions,
http2.SecureServerOptions {
protocol: 'https2';
Copy link
Member

@bajtos bajtos May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: can we please use http2s ("http2" + "secure")? I find https2 weird to read.

Suggested change
protocol: 'https2';
protocol: 'http2s';

Note that HTTP/2 spec puts the version number before additional flags too, e.g h2c for clear-text connections (not hc2). See e.g. https://stackoverflow.com/a/37373039/69868

Similarly the interface should be called Http2SecureOptions or perhaps Http2sOptions if you prefer.

}

/**
* Possible server options
*
*/
export type HttpServerOptions = HttpOptions | HttpsOptions;
export type HttpServerOptions =
| HttpOptions
| HttpsOptions
| Http2Options
| Https2Options;

/**
* Supported protocols
*
*/
export type HttpProtocol = 'http' | 'https'; // Will be extended to `http2` in the future
export type HttpProtocol = 'http' | 'https' | 'http2' | 'https2';

/**
* HTTP / HTTPS server used by LoopBack's RestServer
Expand All @@ -71,8 +100,12 @@ export class HttpServer {
private _listening = false;
private _protocol: HttpProtocol;
private _address: string | AddressInfo;
private requestListener: RequestListener;
readonly server: http.Server | https.Server;
private requestListener: RequestListener | Http2RequestListener;
readonly server:
bajtos marked this conversation as resolved.
Show resolved Hide resolved
| http.Server
| https.Server
| http2.Http2Server
| http2.Http2SecureServer;
private _stoppable: stoppable.StoppableServer;
private serverOptions: HttpServerOptions;

Expand All @@ -81,7 +114,7 @@ export class HttpServer {
* @param serverOptions
*/
constructor(
requestListener: RequestListener,
requestListener: RequestListener | Http2RequestListener,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this constructor API sub-optimal, because it allows caller to mix HTTP v1 request listener with HTTP v2 server options and vice versa.

Can we use function overloads to implement more tight type checks please?

class HttpServer {
  // ...
  constructor(
     requestListener: RequestListener,
     serverOptions?: HttpOptions | HttpsOptions
  );

  constructor(
     requestListener: Http2RequestListener,
     serverOptions?: Http2Options | Https2Options
  );

  constructor(
    requestListener: RequestListener | Http2RequestListener,
    serverOptions?: HttpServerOptions,
  ) {
    // the implementation
  }
}

serverOptions?: HttpServerOptions,
) {
this.requestListener = requestListener;
Expand All @@ -95,19 +128,47 @@ export class HttpServer {
// Remove `port` so that `path` is honored
delete this.serverOptions.port;
}
this._protocol = serverOptions ? serverOptions.protocol ?? 'http' : 'http';
if (this._protocol === 'https') {
this.server = https.createServer(
this.serverOptions as https.ServerOptions,
this.requestListener,
);
} else {
this.server = http.createServer(this.requestListener);
this._protocol = serverOptions?.protocol ?? 'http';
switch (this._protocol) {
case 'https':
this.server = https.createServer(
this.serverOptions as https.ServerOptions,
this.requestListener as RequestListener,
);
break;
case 'http':
this.server = http.createServer(
this.requestListener as RequestListener,
);
break;
case 'http2':
this.server = http2.createServer(
this.requestListener as Http2RequestListener,
);
break;
case 'https2':
this.server = http2.createSecureServer(
this.requestListener as Http2RequestListener,
);
break;
}
// Set up graceful stop for http server
if (typeof this.serverOptions.gracePeriodForClose === 'number') {
let serverNormalized;

switch (this._protocol) {
case 'http':
case 'http2':
serverNormalized = this.server as http.Server;
break;
case 'https':
case 'https2':
serverNormalized = this.server as https.Server;
break;
}

this._stoppable = stoppable(
this.server,
serverNormalized,
this.serverOptions.gracePeriodForClose,
);
}
Expand Down
17 changes: 14 additions & 3 deletions packages/testlab/src/http-server-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// License text available at https://opensource.org/licenses/MIT

import {readFileSync} from 'fs';
import http2 from 'http2';
import {ServerOptions as HttpsServerOptions} from 'https';
import {ListenOptions} from 'net';
import path from 'path';
Expand All @@ -22,6 +23,16 @@ export interface HttpsOptions extends ListenOptions, HttpsServerOptions {
protocol: 'https';
}

export interface Http2Options extends ListenOptions, http2.ServerOptions {
protocol: 'http2';
}

export interface Http2sOptions
extends ListenOptions,
http2.SecureServerOptions {
protocol: 'http2s';
}

export type HostPort = {
host: string;
port: number;
Expand All @@ -36,9 +47,9 @@ export type HostPort = {
*
* @param customConfig - Additional configuration options to apply.
*/
export function givenHttpServerConfig<T extends HttpOptions | HttpsOptions>(
customConfig?: T,
): HostPort & T {
export function givenHttpServerConfig<
T extends HttpOptions | HttpsOptions | Http2Options | Http2sOptions
>(customConfig?: T): HostPort & T {
const defaults = {
host: '127.0.0.1',
port: 0,
Expand Down