Skip to content

Commit

Permalink
HostRouter ignoreCase config (#428)
Browse files Browse the repository at this point in the history
This PR adds a config `ignoreCase` to `HostRouter`, which defaults to
`true` and does what you probably expect (which is what webservers are
generally expected to do and it was a surprise that we didn't).
  • Loading branch information
danfuzz authored Oct 31, 2024
2 parents 44de3a2 + e8be9ea commit fdb1c85
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 17 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ Breaking changes:
* Changed "human" logs to just emit a "seconds-only" timestamp on each logged
event, while adding a full timestamp as a header of sorts once per minute.
This makes for more width for the logged payloads, so easier to read.
* `webapp-builtins`:
* Added `ignoreCase` option to `HostRouter`, which defaults to `true`. (This
is a breaking change, because it never used to ignore case, which was
surprising.)

Other notable changes:
* None.
Expand Down
5 changes: 5 additions & 0 deletions doc/configuration/4-built-in-applications.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ following configuration bindings:
the _names_ of other applications as values. A wildcard only covers the prefix
of a hostname and cannot be used for hostnames identified by numeric IP
address.
* `ignoreCase` — Optional boolean indicating whether (`true`) or not
(`false`) the case of hostnames should be ignored and always looked up as
lowercase. Default `true`, which is the generally-accepted behavior of
websites. **Note:** This setting does not affect the hostname as received by
applications.

**Note:** Unlike `PathRouter`, this application does not do fallback to
less-and-less specific routes; it just finds (at most) one to route to.
Expand Down
7 changes: 4 additions & 3 deletions etc/example-setup/config/config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,12 @@ const applications = [
]
},
{
name: 'myHosts',
class: HostRouter,
name: 'myHosts',
class: HostRouter,
ignoreCase: true,
hosts: {
'*': 'myPaths',
'127.0.0.1': 'mySeries'
'127.0.0.1': 'mySeries',
}
},
{
Expand Down
31 changes: 30 additions & 1 deletion src/net-util/export/HostInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ export class HostInfo {
*/
#nameKey = null;

/**
* The result of {@link #toLowerCase}, or `null` if not yet calculated.
*
* @type {HostInfo}
*/
#lowercaseVersion = null;

/**
* Constructs an instance.
*
Expand All @@ -58,7 +65,9 @@ export class HostInfo {
* se or a string.
*/
constructor(nameString, portNumber) {
this.#nameString = MustBe.string(nameString, /./);
// Note: The regex is a bit lenient. TODO: Maybe it shouldn't be?
this.#nameString = MustBe.string(nameString, /^[-_.:[\]a-zA-Z0-9]+$/);

this.#portNumber = AskIf.string(portNumber)
? Number(MustBe.string(portNumber, /^[0-9]+$/))
: MustBe.number(portNumber);
Expand Down Expand Up @@ -139,6 +148,26 @@ export class HostInfo {
return this.#nameIsIp;
}

/**
* Gets an instance of this class which is identical to `this` but with the
* name lowercased. If this instance's name is already all-lowercase, then
* this method returns `this`.
*
* @returns {HostInfo} The lowercased version.
*/
toLowerCase() {
if (this.#lowercaseVersion === null) {
const name = this.#nameString;
const lowerName = name.toLowerCase();

this.#lowercaseVersion = (name === lowerName)
? this
: new HostInfo(lowerName, this.#portNumber);
}

return this.#lowercaseVersion;
}


//
// Static members
Expand Down
25 changes: 25 additions & 0 deletions src/net-util/tests/HostInfo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,31 @@ describe('nameIsIpAddress()', () => {
});
});

describe('toLowerCase()', () => {
test('returns `this` if the name is already all-lowercase', () => {
const hi = new HostInfo('fleep.florp', 123);
expect(hi.toLowerCase()).toBe(hi);
});

test('returns a correct new instance if the name needs to be lowercased', () => {
const hi = new HostInfo('fleEP.florp', 123);
const got = hi.toLowerCase();

expect(got).not.toBe(hi);
expect(got.portNumber).toBe(123);
expect(got.nameString).toBe('fleep.florp');
});

test('returns the same not-`this` result on subsequent calls', () => {
const hi = new HostInfo('fleep.florP', 123);
const got1 = hi.toLowerCase();
const got2 = hi.toLowerCase();

expect(got1).not.toBe(hi);
expect(got2).toBe(got1);
});
});

describe('localhostInstance()', () => {
describe.each`
port
Expand Down
74 changes: 61 additions & 13 deletions src/webapp-builtins/export/HostRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { TreeMap } from '@this/collections';
import { Names } from '@this/compy';
import { HostUtil, IntfRequestHandler } from '@this/net-util';
import { HostInfo, HostUtil, IntfRequestHandler } from '@this/net-util';
import { MustBe } from '@this/typey';
import { BaseApplication } from '@this/webapp-core';

Expand All @@ -13,6 +13,13 @@ import { BaseApplication } from '@this/webapp-core';
* equivalent). See docs for configuration object details.
*/
export class HostRouter extends BaseApplication {
/**
* Same value as in the config object.
*
* @type {boolean}
*/
#ignoreCase = null;

/**
* Map which goes from a host prefix to a handler (typically a
* {@link BaseApplication}) which should handle that prefix. Gets set in
Expand All @@ -26,15 +33,13 @@ export class HostRouter extends BaseApplication {

/** @override */
async _impl_handleRequest(request, dispatch) {
const host = request.host;
const found = this.#routeTree.find(host.nameKey);
const host = request.host;
const application = this.#applicationFromHost(host);

if (!found) {
if (!application) {
return null;
}

const application = found.value;

dispatch.logger?.dispatchingHost({
application: application.name,
host: host.namePortString
Expand All @@ -61,19 +66,36 @@ export class HostRouter extends BaseApplication {
// the case that all of the referenced apps have already been added when
// that runs.

const { hosts, ignoreCase } = this.config;

const appManager = this.root.applicationManager;
const routeTree = new TreeMap();

for (const [host, name] of this.config.hosts) {
for (const [host, name] of hosts) {
const app = appManager.get(name);
routeTree.add(host, app);
}

this.#routeTree = routeTree;
this.#ignoreCase = ignoreCase;
this.#routeTree = routeTree;

await super._impl_start();
}

/**
* Finds the application for the given host, if any.
*
* @param {HostInfo} host Host info.
* @returns {?BaseApplication} The application, or `null` if there was no
* match.
*/
#applicationFromHost(host) {
const key = this.#ignoreCase ? host.toLowerCase().nameKey : host.nameKey;
const found = this.#routeTree.find(key);

return found?.value ?? null;
}


//
// Static members
Expand All @@ -95,15 +117,41 @@ export class HostRouter extends BaseApplication {
_config_hosts(value) {
MustBe.plainObject(value);

const result = new TreeMap();

for (const [host, name] of Object.entries(value)) {
Names.checkName(name);
const key = HostUtil.parseHostname(host, true);
result.add(key, name);
HostUtil.checkHostname(host, true);
}

return value;
}

/**
* Should the case of hostnames be ignored (specifically, folded to
* lowercase)?
*
* @param {boolean} [value] Proposed configuration value.
* @returns {boolean} Accepted configuration value.
*/
_config_ignoreCase(value = true) {
return MustBe.boolean(value);
}

/** @override */
_impl_validate(config) {
// We can (and do) only create the `hosts` map here, after we know the
// value for `ignoreCase`.

const { hosts: hostsObj, ignoreCase } = config;
const hosts = new TreeMap();

for (const [host, name] of Object.entries(hostsObj)) {
const keyString = ignoreCase ? host.toLowerCase() : host;
const key = HostUtil.parseHostname(keyString, true);
hosts.add(key, name);
}
Object.freeze(hosts);

return Object.freeze(result);
return { ...config, hosts };
}
};
}
Expand Down
74 changes: 74 additions & 0 deletions src/webapp-builtins/tests/HostRouter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ describe('constructor', () => {
})).not.toThrow();
});

test('accepts a valid minimal configuration with `ignoreCase`', () => {
expect(() => new HostRouter({
hosts: {},
ignoreCase: false
})).not.toThrow();
});

test('accepts a valid configuration with several non-wildcard hosts', () => {
expect(() => new HostRouter({
hosts: {
Expand Down Expand Up @@ -53,6 +60,42 @@ describe('constructor', () => {
})).not.toThrow();
});

test('does not allow two names that differ only in case when `ignoreCase === true`', () => {
expect(() => new HostRouter({
ignoreCase: true,
hosts: {
'Boop.bop': 'app1',
'booP.bop': 'app2'
}
})).toThrow();

expect(() => new HostRouter({
ignoreCase: true,
hosts: {
'*.ZONK': 'app1',
'*.ZoNK': 'app2'
}
})).toThrow();
});

test('allows two names that differ only in case when `ignoreCase === false`', () => {
expect(() => new HostRouter({
ignoreCase: false,
hosts: {
'Boop.bop': 'app1',
'booP.bop': 'app2'
}
})).not.toThrow();

expect(() => new HostRouter({
ignoreCase: false,
hosts: {
'*.ZONK': 'app1',
'*.ZoNK': 'app2'
}
})).not.toThrow();
});

test.each`
name
${undefined}
Expand Down Expand Up @@ -225,6 +268,37 @@ describe('_impl_handleRequest()', () => {
await expectApp(hr, 'zorch.splat', 'mockApp1');
});

test('routes to a case-folded DNS name when `ignoreCase === true`', async () => {
const hr = await makeInstance(
{
ignoreCase: true,
hosts: {
'splat': 'mockApp2',
'ZORCH.splat': 'mockApp1',
'blurp.zorch.splat': 'mockApp2'
}
},
{ appCount: 2 }
);

await expectApp(hr, 'zorch.SPLAT', 'mockApp1');
});

test('routes to a case-matched DNS name when `ignoreCase === false`', async () => {
const hr = await makeInstance(
{
ignoreCase: false,
hosts: {
'ZORCH.splat': 'mockApp1',
'zorch.SPLAT': 'mockApp2'
}
},
{ appCount: 2 }
);

await expectApp(hr, 'zorch.SPLAT', 'mockApp2');
});

test('does not route to an exact-match DNS name as if it were a wildcard', async () => {
const hr = await makeInstance({
hosts: {
Expand Down

0 comments on commit fdb1c85

Please sign in to comment.