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.

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

Allow markbind serve to specify custom host #2382 #2395

Merged
merged 12 commits into from
Feb 17, 2024
8 changes: 5 additions & 3 deletions docs/userGuide/cliCommands.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Usage: markbind <command>

**Description:** Does the following steps:
1. Builds the site and puts the generated files in a directory named `_site`.
1. Starts a web server instance locally and makes the site available at `http://127.0.0.1:8080`.
1. Starts a web server instance locally and makes the site available at `http://127.0.0.1:8080` by default.
1. Opens a <trigger trigger="click" for="modal:cliCommands-livePreview">live preview</trigger> of the website.

<modal large header="Live Preview" id="modal:cliCommands-livePreview">
Expand Down Expand Up @@ -124,9 +124,11 @@ The caveat is that not building all pages during the initial process, or not reb
* `-f`, `--force-reload`<br>
Force live reload to process all files in the site, instead of just the relevant files. This option is useful when you are modifying a file that is not a file type monitored by the <trigger trigger="click" for="modal:cliCommands-livePreview">live preview</trigger> feature.

* `-p <port>`, `--port <port>`<br>
Serve the website in the specified port.
* `-a <address>`, `--address <address>`<br>
Specify the server address/host (Default is 127.0.0.1).

* `-p <port>`, `--port <port>`<br>
Serve the website in the specified port (Default is 8080).

{{ icon_examples }}
* `markbind serve` : Serves the site from the current working directory.
Expand Down
1 change: 1 addition & 0 deletions packages/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ program
.option('-p, --port <port>', 'port for server to listen on (Default is 8080)')
.option('-s, --site-config <file>', 'specify the site config file (default: site.json)')
.option('-d, --dev', 'development mode, enabling live & hot reload for frontend source files.')
.option('-a, --address <address>', 'specify the server address/host (Default is 127.0.0.1)')
.description('build then serve a website from a directory')
.action((userSpecifiedRoot, options) => {
serve(userSpecifiedRoot, options);
Expand Down
37 changes: 35 additions & 2 deletions packages/cli/src/cmd/serve.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const chokidar = require('chokidar');
const path = require('path');
const readline = require('readline');

const { Site } = require('@markbind/core').Site;
const { pageVueServerRenderer } = require('@markbind/core/src/Page/PageVueServerRenderer');
Expand All @@ -17,6 +18,23 @@ const {
removeHandler,
} = require('../util/serveUtil');

function isIPAddressZero(address) {
const patternForZero = /^0(\.0)*$/;

return patternForZero.test(address);
}

function questionAsync(question) {
const readlineInterface = readline.createInterface({ input: process.stdin, output: process.stdout });

return new Promise((resolve) => {
readlineInterface.question(question, (response) => {
readlineInterface.close();
resolve(response);
});
});
}

function serve(userSpecifiedRoot, options) {
if (options.dev) {
logger.useDebugConsole();
Expand Down Expand Up @@ -63,13 +81,27 @@ function serve(userSpecifiedRoot, options) {
logLevel: 0,
root: outputFolder,
port: options.port || 8080,
host: options.address || '127.0.0.1',
middleware: [],
mount: [],
};

site
.readSiteConfig()
.then(async (config) => {
if (isIPAddressZero(serverConfig.host)) {
const response = await questionAsync(
'WARNING: Using the address \'0.0.0.0\' could potentially expose your server to the internet, '
+ 'which may pose security risks. \n'
+ 'Proceed with caution? [y/N] ');
itsyme marked this conversation as resolved.
Show resolved Hide resolved
if (response.toLowerCase() === 'y') {
logger.info('Proceeding to generate website');
} else {
logger.info('Website generation is cancelled.');
process.exit();
}
}

serverConfig.mount.push([config.baseUrl || '/', outputFolder]);

if (options.dev) {
Expand Down Expand Up @@ -111,8 +143,9 @@ function serve(userSpecifiedRoot, options) {
const server = liveServer.start(serverConfig);
server.addListener('listening', () => {
const address = server.address();
const serveHost = address.address === '0.0.0.0' ? '127.0.0.1' : address.address;
const serveURL = `http://${serveHost}:${address.port}`;
const serveHost = address.address;
Copy link
Contributor

@itsyme itsyme Feb 10, 2024

Choose a reason for hiding this comment

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

What do you think about adding some validation to the user input host address?

I believe that this could be better for UX as

  • it checks the host address before building the pages (which could take some time)
  • it better handles the errors that are being thrown. Currently users will be exposed to errors like Error: getaddrinfo ENOTFOUND 127.0.300.1

I was thinking some sort of simple validation like checking if the input is an actual IP address and if any number in the address exceeds 255 could improve the user experience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, good idea! I was thinking if we want to do this validation, we can actually just do the validations (including check if port is unavailable, check if host is being used etc) all at once before building the pages. Currently, all these validations are done after building the pages.

Since this PR is focusing on adding another small feature, I suggest we create a separate issue and pull request to specifically address this validation issue. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep I think a separate issue is fine as well! I believe there is some level of error handling for when the host is being used. From my experience if I specified a used host, the nearest unused hostname will be used, which allows the user to still serve the site. I think we can discuss about what needs to be validated once the issue is opened!

const servePort = address.port;
const serveURL = `http://${serveHost}:${servePort}`;
logger.info(`Serving "${outputFolder}" at ${serveURL}`);
logger.info('Press CTRL+C to stop ...');
});
Expand Down
18 changes: 12 additions & 6 deletions packages/cli/src/lib/live-server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function entryPoint(staticHandler, file) {

/**
* Start a live server with parameters given as an object
* @param host {string} Address to bind to (default: 0.0.0.0)
* @param host {string} Address to bind to (default: 127.0.0.1)
* @param port {number} Port number (default: 8080)
* @param root {string} Path to root directory (default: cwd)
* @param watch {array} Paths to exclusively watch for changes
Expand All @@ -158,8 +158,8 @@ function entryPoint(staticHandler, file) {
*/
LiveServer.start = function(options) {
options = options || {};
var host = options.host || '0.0.0.0';
var port = options.port !== undefined ? options.port : 8080; // 0 means random
var host = options.host ?? '127.0.0.1';
var port = options.port ?? 8080; // 0 means random
var root = options.root || process.cwd();
var mount = options.mount || [];
var watchPaths = options.watch || [root];
Expand Down Expand Up @@ -281,7 +281,13 @@ LiveServer.start = function(options) {
setTimeout(function() {
server.listen(0, host);
}, 1000);
} else {
} else if (e.code === 'EADDRNOTAVAIL') {
console.log('%s is not available. Trying another address'.yellow, host);
setTimeout(function() {
server.listen(port, '127.0.0.1');
}, 1000);
}
else {
console.error(e.toString().red);
LiveServer.shutdown();
}
Expand All @@ -292,8 +298,8 @@ LiveServer.start = function(options) {
LiveServer.server = server;

var address = server.address();
var serveHost = address.address === "0.0.0.0" ? "127.0.0.1" : address.address;
var openHost = host === "0.0.0.0" ? "127.0.0.1" : host;
var serveHost = address.address;
var openHost = host;

var serveURL = protocol + '://' + serveHost + ':' + address.port;
var openURL = protocol + '://' + openHost + ':' + address.port;
Expand Down
Loading