Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

User can now specify a port for the live dev server #6815

Merged
merged 5 commits into from
Feb 14, 2014
Merged

User can now specify a port for the live dev server #6815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ce04d9
User can now specify a port for the live dev server
Feb 10, 2014
f0a19e3
Merge remote-tracking branch 'upstream/master' into livedev-port
Feb 12, 2014
d89c842
pull port pref at time of getServer instead of at brackets load
Feb 12, 2014
d5dd7b8
Merge remote-tracking branch 'upstream/master' into livedev-port
Feb 12, 2014
5df149d
Refactored to detect port changes between LiveDev launches.
Feb 12, 2014
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
58 changes: 50 additions & 8 deletions src/extensions/default/StaticServer/StaticServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,17 @@ maxerr: 50, browser: true */
define(function (require, exports, module) {
"use strict";

var BaseServer = brackets.getModule("LiveDevelopment/Servers/BaseServer").BaseServer,
FileUtils = brackets.getModule("file/FileUtils");
var BaseServer = brackets.getModule("LiveDevelopment/Servers/BaseServer").BaseServer,
FileUtils = brackets.getModule("file/FileUtils"),
PreferencesManager = brackets.getModule("preferences/PreferencesManager");


/**
* @private
*
* Prefences manager for this extension
*/
var _prefs = PreferencesManager.getExtensionPrefs("staticserver");

/**
* @constructor
Expand All @@ -48,7 +57,7 @@ define(function (require, exports, module) {
function StaticServer(config) {
this._nodeDomain = config.nodeDomain;
this._onRequestFilter = this._onRequestFilter.bind(this);

BaseServer.call(this, config);
}

Expand Down Expand Up @@ -102,13 +111,46 @@ define(function (require, exports, module) {
*/
StaticServer.prototype.readyToServe = function () {
var self = this;
return this._nodeDomain.exec("getServer", self._root)
var deferred = new $.Deferred();

function sanitizePort(port) {
port = parseInt(port, 10);
port = (port && !isNaN(port) && port > 0 && port < 65536) ? port : 0;
return port;
}

function onSuccess(address) {
self._baseUrl = "http://" + address.address + ":" + address.port + "/";
deferred.resolve();
}

function onFailure() {
self._baseUrl = "";
deferred.resolve();
}

var port = sanitizePort(_prefs.get("port"));

this._nodeDomain.exec("getServer", self._root, port)
.done(function (address) {
self._baseUrl = "http://" + address.address + ":" + address.port + "/";

// If the port returned wasn't what was requested, then the preference has
// changed. Close the current server, and open a new one with the new port.
if (address.port !== port && port > 0) {
return self._nodeDomain.exec("closeServer", self._root)
.done(function () {
return self._nodeDomain.exec("getServer", self._root, port)
.done(onSuccess)
.fail(onFailure);
})
.fail(onFailure);
}

onSuccess(address);
})
.fail(function () {
self._baseUrl = "";
});
.fail(onFailure);

return deferred.promise();
};

/**
Expand Down
4 changes: 2 additions & 2 deletions src/extensions/default/StaticServer/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ define(function (require, exports, module) {
* @type {NodeDomain}
*/
var _nodeDomain = new NodeDomain("staticServer", _domainPath);

/**
* @private
* @return {StaticServerProvider} The singleton StaticServerProvider initialized
Expand All @@ -64,7 +64,7 @@ define(function (require, exports, module) {

return new StaticServer(config);
}

AppInit.appReady(function () {
LiveDevServerManager.registerServer({ create: _createStaticServer }, 5);
});
Expand Down
59 changes: 47 additions & 12 deletions src/extensions/default/StaticServer/node/StaticServerDomain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,20 @@
function normalizeRootPath(path) {
return (path && path[path.length - 1] === "/") ? path.slice(0, -1) : path;
}


/**
* @private
* Converts given value to a valid port number or zero.
* A zero will cause a random port to be used.
* @param {number} port number to clean, can be string as well
* @return {number} a valid port number or zero if a value wasn't passed in or valid.
*/
function sanitizePort(port) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it's probably safe to just do this on one side of the connection. You're now doing this on the client side. Or is there a code path I haven't noticed yet that could still pass an unsanitized port across?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vacillated on that and just decided to have them both in, but I agree
that it's probably safe to not duplicate it. I wanted it in StaticServer
to clean up any invalid pref values which was necessary for detecting port
changes. I would get rid of it on the StaticServerDomain side since
currently there is nothing else calling it.

On Thu, Feb 13, 2014 at 3:16 PM, Kevin Dangoor notifications@github.comwrote:

In src/extensions/default/StaticServer/node/StaticServerDomain.js:

@@ -98,7 +98,20 @@
function normalizeRootPath(path) {
return (path && path[path.length - 1] === "/") ? path.slice(0, -1) : path;

}

  • /**
  • \* @private

  • \* Converts given value to a valid port number or zero.

  • \* A zero will cause a random port to be used.

  • \* @param {number} port number to clean, can be string as well

  • \* @return {number} a valid port number or zero if a value wasn't passed in or valid.

  • */
  • function sanitizePort(port) {

It seems like it's probably safe to just do this on one side of the
connection. You're now doing this on the client side. Or is there a code
path I haven't noticed yet that could still pass an unsanitized port across?

Reply to this email directly or view it on GitHubhttps://github.com//pull/6815/files#r9727721
.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I can do that easily when I'm testing before I merge.

port = parseInt(port, 10);
port = (port && !isNaN(port) && port > 0 && port < 65536) ? port : 0;
return port;
}

/**
* @private
* Generates a key based on a server's absolute path
Expand All @@ -117,10 +130,11 @@
* an error (or null if there was no error) and the server (or null if there
* was an error).
*/
function _createServer(path, createCompleteCallback) {
function _createServer(path, port, createCompleteCallback) {
var server,
app,
address,
portNum = sanitizePort(port),
pathKey = getPathKey(path);

// create a new map for this server's requests
Expand Down Expand Up @@ -208,11 +222,11 @@

// dispatch request event
_domainManager.emitEvent("staticServer", "requestFilter", [request]);

// set a timeout if custom responses are not returned
timeoutId = setTimeout(function () { resume(true); }, _filterRequestTimeout);
}

app = connect();
app.use(rewrite);
// JSLint complains if we use `connect.static` because static is a
Expand All @@ -221,7 +235,10 @@
app.use(connect.directory(path));

server = http.createServer(app);
server.listen(0, "127.0.0.1", function () {

// Once the server is listening then verify we can handle requests
// before calling the callback
server.on("listening", function () {
requestRoot(
server,
function (err, res) {
Expand All @@ -233,6 +250,17 @@
}
);
});

// If the given port/address is in use then use a random port
server.on("error", function (e) {
if (e.code === "EADDRINUSE") {
server.listen(0, "127.0.0.1");
} else {
throw e;
}
});

server.listen(portNum, "127.0.0.1");
}

/**
Expand All @@ -248,13 +276,13 @@
* The "family" property of the address indicates whether the address is,
* for example, IPv4, IPv6, or a UNIX socket.
*/
function _cmdGetServer(path, cb) {
function _cmdGetServer(path, port, cb) {
// Make sure the key doesn't conflict with some built-in property of Object.
var pathKey = getPathKey(path);
if (_servers[pathKey]) {
cb(null, _servers[pathKey].address());
} else {
_createServer(path, function (err, server) {
_createServer(path, port, function (err, server) {
if (err) {
cb(err, null);
} else {
Expand Down Expand Up @@ -371,11 +399,18 @@
_cmdGetServer,
true,
"Starts or returns an existing server for the given path.",
[{
name: "path",
type: "string",
description: "absolute filesystem path for root of server"
}],
[
{
name: "path",
type: "string",
description: "Absolute filesystem path for root of server."
},
{
name: "port",
type: "number",
description: "Port number to use for HTTP server. Pass zero to assign a random port."
}
],
[{
name: "address",
type: "{address: string, family: string, port: number}",
Expand Down
76 changes: 63 additions & 13 deletions src/extensions/default/StaticServer/unittests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ define(function (require, exports, module) {
it("should start a static server on the given folder", function () {
var serverInfo, path = testFolder + "/folder1";
runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand All @@ -110,10 +110,59 @@ define(function (require, exports, module) {
});
});

it("should start the server on the given port", function () {
var serverInfo,
path = testFolder + "/folder1";

runs(function () {
nodeConnection.domains.staticServer.getServer(path, 54321)
.done(function (info) {
serverInfo = info;
});
});

waitsFor(function () { return serverInfo; }, "waiting for static server to start");
runs(function () {
expect(serverInfo.port).toBe(54321);

waitsForDone(nodeConnection.domains.staticServer.closeServer(path),
"waiting for static server to close");
});
});

it("should start a static server using a random port when the given port is already in use", function () {
var serverInfo1, serverInfo2,
path1 = testFolder + "/folder1", path2 = testFolder + "/folder2";

runs(function () {
nodeConnection.domains.staticServer.getServer(path1, 54321)
.done(function (info) {
serverInfo1 = info;
});
nodeConnection.domains.staticServer.getServer(path2, 54321)
.done(function (info) {
serverInfo2 = info;
});
});

waitsFor(function () { return serverInfo1 && serverInfo2; }, "waiting for static servers to start");

runs(function () {
expect(serverInfo1.port).toBe(54321);
expect(serverInfo2.port).not.toBe(54321);
expect(serverInfo2.port).toBeGreaterThan(0);

waitsForDone(nodeConnection.domains.staticServer.closeServer(path1),
"waiting for static server 1 to close");
waitsForDone(nodeConnection.domains.staticServer.closeServer(path2),
"waiting for static server 2 to close");
});
});

it("should serve the text of a file in the given folder", function () {
var serverInfo, text, path = testFolder + "/folder1";
runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -141,11 +190,11 @@ define(function (require, exports, module) {
var serverInfo1, serverInfo2,
path1 = testFolder + "/folder1", path2 = testFolder + "/folder2";
runs(function () {
nodeConnection.domains.staticServer.getServer(path1)
nodeConnection.domains.staticServer.getServer(path1, 0)
.done(function (info) {
serverInfo1 = info;
});
nodeConnection.domains.staticServer.getServer(path2)
nodeConnection.domains.staticServer.getServer(path2, 0)
.done(function (info) {
serverInfo2 = info;
});
Expand All @@ -168,11 +217,11 @@ define(function (require, exports, module) {
path1 = testFolder + "/folder1", path2 = testFolder + "/folder2",
text1, text2;
runs(function () {
nodeConnection.domains.staticServer.getServer(path1)
nodeConnection.domains.staticServer.getServer(path1, 0)
.done(function (info) {
serverInfo1 = info;
});
nodeConnection.domains.staticServer.getServer(path2)
nodeConnection.domains.staticServer.getServer(path2, 0)
.done(function (info) {
serverInfo2 = info;
});
Expand Down Expand Up @@ -212,7 +261,7 @@ define(function (require, exports, module) {
timeout = 500;

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -268,7 +317,7 @@ define(function (require, exports, module) {
requestId;

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -318,7 +367,7 @@ define(function (require, exports, module) {
requestId;

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -365,7 +414,7 @@ define(function (require, exports, module) {
requestId;

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -427,7 +476,7 @@ define(function (require, exports, module) {
});

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -486,7 +535,7 @@ define(function (require, exports, module) {
timeout = 500;

runs(function () {
nodeConnection.domains.staticServer.getServer(path)
nodeConnection.domains.staticServer.getServer(path, 0)
.done(function (info) {
serverInfo = info;
});
Expand Down Expand Up @@ -549,7 +598,8 @@ define(function (require, exports, module) {
baseUrl: "http://localhost/",
nodeDomain: mockNodeDomain,
pathResolver: pathResolver,
root: projectPath
root: projectPath,
port: 0
};

it("should translate local paths to server paths", function () {
Expand Down