Skip to content

refactor(server): clean up lib/server.js #1873

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

Merged
merged 3 commits into from
May 15, 2019
Merged

refactor(server): clean up lib/server.js #1873

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84eb4d6
refactor(server): clean up lib/server.js
hiroppy May 15, 2019
5f086fe
Merge branch 'master' into feature/refactor-server
evilebottnawi May 15, 2019
e1fd450
Merge branch 'master' into feature/refactor-server
hiroppy May 15, 2019
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
233 changes: 119 additions & 114 deletions lib/Server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,6 @@ if (semver.satisfies(process.version, '8.6.0 - 9')) {
}

class Server {
static get DEFAULT_STATS() {
return {
all: false,
hash: true,
assets: true,
warnings: true,
errors: true,
errorDetails: false,
};
}

constructor(compiler, options = {}, _log) {
if (options.lazy && !options.filename) {
throw new Error("'filename' option must be set in lazy mode.");
Expand Down Expand Up @@ -142,108 +131,8 @@ class Server {
this.websocketProxies = [];

this.setupFeatures();

// if the user enables http2, we can safely enable https
if (this.options.http2 && !this.options.https) {
this.options.https = true;
}

if (this.options.https) {
// for keep supporting CLI parameters
if (typeof this.options.https === 'boolean') {
this.options.https = {
ca: this.options.ca,
pfx: this.options.pfx,
key: this.options.key,
cert: this.options.cert,
passphrase: this.options.pfxPassphrase,
requestCert: this.options.requestCert || false,
};
}

for (const property of ['ca', 'pfx', 'key', 'cert']) {
const value = this.options.https[property];
const isBuffer = value instanceof Buffer;

if (value && !isBuffer) {
let stats = null;

try {
stats = fs.lstatSync(fs.realpathSync(value)).isFile();
} catch (error) {
// ignore error
}

if (stats) {
// It is file
this.options.https[property] = fs.readFileSync(path.resolve(value));
} else {
this.options.https[property] = value;
}
}
}

let fakeCert;

if (!this.options.https.key || !this.options.https.cert) {
fakeCert = getCertificate(this.log);
}

this.options.https.key = this.options.https.key || fakeCert;
this.options.https.cert = this.options.https.cert || fakeCert;

// Only prevent HTTP/2 if http2 is explicitly set to false
const isHttp2 = this.options.http2 !== false;

// note that options.spdy never existed. The user was able
// to set options.https.spdy before, though it was not in the
// docs. Keep options.https.spdy if the user sets it for
// backwards compatability, but log a deprecation warning.
if (this.options.https.spdy) {
// for backwards compatability: if options.https.spdy was passed in before,
// it was not altered in any way
this.log.warn(
'Providing custom spdy server options is deprecated and will be removed in the next major version.'
);
} else {
// if the normal https server gets this option, it will not affect it.
this.options.https.spdy = {
protocols: ['h2', 'http/1.1'],
};
}

// `spdy` is effectively unmaintained, and as a consequence of an
// implementation that extensively relies on Node’s non-public APIs, broken
// on Node 10 and above. In those cases, only https will be used for now.
// Once express supports Node's built-in HTTP/2 support, migrating over to
// that should be the best way to go.
// The relevant issues are:
// - https://github.com/nodejs/node/issues/21665
// - https://github.com/webpack/webpack-dev-server/issues/1449
// - https://github.com/expressjs/express/issues/3388
if (semver.gte(process.version, '10.0.0') || !isHttp2) {
if (this.options.http2) {
// the user explicitly requested http2 but is not getting it because
// of the node version.
this.log.warn(
'HTTP/2 is currently unsupported for Node 10.0.0 and above, but will be supported once Express supports it'
);
}
this.listeningApp = https.createServer(this.options.https, this.app);
} else {
/* eslint-disable global-require */
// The relevant issues are:
// https://github.com/spdy-http2/node-spdy/issues/350
// https://github.com/webpack/webpack-dev-server/issues/1592
this.listeningApp = require('spdy').createServer(
this.options.https,
this.app
);
/* eslint-enable global-require */
}
} else {
this.listeningApp = http.createServer(this.app);
}
this.setupHttps();
this.createServer();

killable(this.listeningApp);

Expand Down Expand Up @@ -625,6 +514,122 @@ class Server {
});
}

setupHttps() {
// if the user enables http2, we can safely enable https
if (this.options.http2 && !this.options.https) {
this.options.https = true;
}

if (this.options.https) {
// for keep supporting CLI parameters
if (typeof this.options.https === 'boolean') {
this.options.https = {
ca: this.options.ca,
pfx: this.options.pfx,
key: this.options.key,
cert: this.options.cert,
passphrase: this.options.pfxPassphrase,
requestCert: this.options.requestCert || false,
};
}

for (const property of ['ca', 'pfx', 'key', 'cert']) {
const value = this.options.https[property];
const isBuffer = value instanceof Buffer;

if (value && !isBuffer) {
let stats = null;

try {
stats = fs.lstatSync(fs.realpathSync(value)).isFile();
} catch (error) {
// ignore error
}

// It is file
this.options.https[property] = stats
? fs.readFileSync(path.resolve(value))
: value;
}
}

let fakeCert;

if (!this.options.https.key || !this.options.https.cert) {
fakeCert = getCertificate(this.log);
}

this.options.https.key = this.options.https.key || fakeCert;
this.options.https.cert = this.options.https.cert || fakeCert;

// note that options.spdy never existed. The user was able
// to set options.https.spdy before, though it was not in the
// docs. Keep options.https.spdy if the user sets it for
// backwards compatibility, but log a deprecation warning.
if (this.options.https.spdy) {
// for backwards compatibility: if options.https.spdy was passed in before,
// it was not altered in any way
this.log.warn(
'Providing custom spdy server options is deprecated and will be removed in the next major version.'
);
} else {
// if the normal https server gets this option, it will not affect it.
this.options.https.spdy = {
protocols: ['h2', 'http/1.1'],
};
}
}
}

createServer() {
if (this.options.https) {
// Only prevent HTTP/2 if http2 is explicitly set to false
const isHttp2 = this.options.http2 !== false;

// `spdy` is effectively unmaintained, and as a consequence of an
// implementation that extensively relies on Node’s non-public APIs, broken
// on Node 10 and above. In those cases, only https will be used for now.
// Once express supports Node's built-in HTTP/2 support, migrating over to
// that should be the best way to go.
// The relevant issues are:
// - https://github.com/nodejs/node/issues/21665
// - https://github.com/webpack/webpack-dev-server/issues/1449
// - https://github.com/expressjs/express/issues/3388
if (semver.gte(process.version, '10.0.0') || !isHttp2) {
if (this.options.http2) {
// the user explicitly requested http2 but is not getting it because
// of the node version.
this.log.warn(
'HTTP/2 is currently unsupported for Node 10.0.0 and above, but will be supported once Express supports it'
);
}
this.listeningApp = https.createServer(this.options.https, this.app);
} else {
// The relevant issues are:
// https://github.com/spdy-http2/node-spdy/issues/350
// https://github.com/webpack/webpack-dev-server/issues/1592
// eslint-disable-next-line global-require
this.listeningApp = require('spdy').createServer(
this.options.https,
this.app
);
}
} else {
this.listeningApp = http.createServer(this.app);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should rename this method on createServer, also we can refactor this on two methods:

  • setupHttps (logic for https)
  • createServer (logic for http.createServer)


static get DEFAULT_STATS() {
return {
all: false,
hash: true,
assets: true,
warnings: true,
errors: true,
errorDetails: false,
};
}

getStats(statsObj) {
const stats = Server.DEFAULT_STATS;

Expand Down Expand Up @@ -693,7 +698,7 @@ class Server {
if (ip.isV4Format(hostname) || ip.isV6Format(hostname)) {
return true;
}
// always allow localhost host, for convience
// always allow localhost host, for convenience
if (hostname === 'localhost') {
return true;
}
Expand Down