fix: refuse an empty or dot path id so a request cannot land on a sibling route

README.md

@@ -188,6 +188,14 @@ const decoded = deserialize(encoded, schema);
 // decoded deep-equals the input object
 ```
 
+## What's new in 2.1.1
+
+Rejects a path segment that would send a read to the wrong route.
+
+### Bug fixes
+
+- An empty id, `.`, or `..` passed to an Explorer path method throws before the request goes out. Percent-encoding leaves a dot segment intact, so the URL parser collapsed it and pointed the read at a different route on the same origin, and an empty id turned a single-item lookup into the list route above it. The guard covers the sixteen methods that take an asset id, collection, schema, template, offer, or account name in the path, and none of the three values is ever a valid one. (#20)
+
 ## What's new in 2.1.0
 
 Validates the builders' numeric parameters and deprecates native asset backing.

src/API/Explorer/index.ts

@@ -32,9 +32,34 @@ export type DataOptions = Array<{key: string, value: any, type?: string}>;
 // Asset ids, collection/schema/template names and account names reach the path
 // straight from the caller. Left raw, a value carrying `/`, `?` or `#` escapes
 // its own segment and rewrites the request target, so every one is encoded
-// where the path is assembled rather than in either fetch branch.
-function encodeSegment(value: string): string {
-    return encodeURIComponent(value);
+// where the path is assembled rather than in either fetch branch. Encoding
+// alone does not cover `.` and `..`: both are unreserved, so they survive
+// encodeURIComponent and the URL parser then collapses the dot segment and
+// aims the request at a different route on the same origin. An empty id
+// leaves a bare trailing slash, which turns a single-item route into the list
+// route above it. None of the three is a valid id, name or account, so the
+// segment builder rejects them before a request goes out.
+//
+// A value that equals a sibling route literal such as "_count" or "payouts" is
+// a valid segment and passes: that request lands on the neighbouring route,
+// which is the caller's error, not a rewrite this helper can detect.
+function encodeSegment(value: string | null | undefined, field: string): string {
+    // A missing value would travel as the literal segment "undefined" or "null"
+    // and read as an API fault instead of the caller's mistake.
+    if (value === null || value === undefined) {
+        throw new Error(`${field} is required`);
+    }
+
+    const segment = String(value);
+
+    if (segment === '' || segment === '.' || segment === '..') {
+        throw new Error(
+            `${field} ${JSON.stringify(segment)} is not a valid path segment: ` +
+            'it is empty or a dot segment, so it would rewrite the request path'
+        );
+    }
+
+    return encodeURIComponent(segment);
 }
 
 function buildDataOptions(options: {[key: string]: any}, data: DataOptions): {[key: string]: any} {
@@ -115,15 +140,15 @@ export default class ExplorerApi {
     }
 
     async getAsset(id: string): Promise<IAsset> {
-        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id), {});
+        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id, 'asset id'), {});
     }
 
     async getAssetStats(id: string): Promise<IAssetStats> {
-        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id) + '/stats', {});
+        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id, 'asset id') + '/stats', {});
     }
 
     async getAssetLogs(id: string, page: number = 1, limit: number = 100, order: string = 'desc'): Promise<ILog[]> {
-        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id) + '/logs', {page, limit, order});
+        return await this.fetchEndpoint('/v1/assets/' + encodeSegment(id, 'asset id') + '/logs', {page, limit, order});
     }
 
     async getCollections(options: CollectionApiParams = {}, page: number = 1, limit: number = 100): Promise<ICollection[]> {
@@ -135,15 +160,15 @@ export default class ExplorerApi {
     }
 
     async getCollection(name: string): Promise<ICollection> {
-        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name), {});
+        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name, 'collection name'), {});
     }
 
     async getCollectionStats(name: string): Promise<ICollectionStats> {
-        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name) + '/stats', {});
+        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name, 'collection name') + '/stats', {});
     }
 
     async getCollectionLogs(name: string, page: number = 1, limit: number = 100, order: string = 'desc'): Promise<ILog[]> {
-        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name) + '/logs', {page, limit, order});
+        return await this.fetchEndpoint('/v1/collections/' + encodeSegment(name, 'collection name') + '/logs', {page, limit, order});
     }
 
     async getSchemas(options: SchemaApiParams = {}, page: number = 1, limit: number = 100): Promise<IApiSchema[]> {
@@ -155,15 +180,15 @@ export default class ExplorerApi {
     }
 
     async getSchema(collection: string, name: string): Promise<IApiSchema> {
-        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection) + '/' + encodeSegment(name), {});
+        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(name, 'schema name'), {});
     }
 
     async getSchemaStats(collection: string, name: string): Promise<ISchemaStats> {
-        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection) + '/' + encodeSegment(name) + '/stats', {});
+        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(name, 'schema name') + '/stats', {});
     }
 
     async getSchemaLogs(collection: string, name: string, page: number = 1, limit: number = 100, order: string = 'desc'): Promise<ILog[]> {
-        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection) + '/' + encodeSegment(name) + '/logs', {page, limit, order});
+        return await this.fetchEndpoint('/v1/schemas/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(name, 'schema name') + '/logs', {page, limit, order});
     }
 
     async getTemplates(options: TemplateApiParams = {}, page: number = 1, limit: number = 100, data: DataOptions = []): Promise<ITemplate[]> {
@@ -175,15 +200,15 @@ export default class ExplorerApi {
     }
 
     async getTemplate(collection: string, id: string): Promise<ITemplate> {
-        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection) + '/' + encodeSegment(id), {});
+        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(id, 'template id'), {});
     }
 
-    async getTemplateStats(collection: string, name: string): Promise<ITemplateStats> {
-        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection) + '/' + encodeSegment(name) + '/stats', {});
+    async getTemplateStats(collection: string, id: string): Promise<ITemplateStats> {
+        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(id, 'template id') + '/stats', {});
     }
 
     async getTemplateLogs(collection: string, id: string, page: number = 1, limit: number = 100, order: string = 'desc'): Promise<ILog[]> {
-        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection) + '/' + encodeSegment(id) + '/logs', {page, limit, order});
+        return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(id, 'template id') + '/logs', {page, limit, order});
     }
 
     async getTransfers(options: TransferApiParams = {}, page: number = 1, limit: number = 100): Promise<ITransfer[]> {
@@ -203,7 +228,7 @@ export default class ExplorerApi {
     }
 
     async getOffer(id: string): Promise<IOffer> {
-        return await this.fetchEndpoint('/v1/offers/' + encodeSegment(id), {});
+        return await this.fetchEndpoint('/v1/offers/' + encodeSegment(id, 'offer id'), {});
     }
 
     async getAccounts(options: AccountApiParams = {}, page: number = 1, limit: number = 100): Promise<Array<{account: string, assets: string}>> {
@@ -219,17 +244,18 @@ export default class ExplorerApi {
     }
 
     async getAccount(account: string, options: GreylistParams & HideOffersParams = {}): Promise<IAccountStats> {
-        return await this.fetchEndpoint('/v1/accounts/' + encodeSegment(account), options);
+        return await this.fetchEndpoint('/v1/accounts/' + encodeSegment(account, 'account'), options);
     }
 
     async getAccountCollection(account: string, collection: string): Promise<IAccountCollectionStats> {
-        return await this.fetchEndpoint('/v1/accounts/' + encodeSegment(account) + '/' + encodeSegment(collection), {});
+        return await this.fetchEndpoint('/v1/accounts/' + encodeSegment(account, 'account') + '/' + encodeSegment(collection, 'collection name'), {});
     }
 
     async getAccountBurns(account: string, options: GreylistParams & HideOffersParams = {}): Promise<IAccountStats> {
-        return await this.fetchEndpoint('/v1/burns/' + encodeSegment(account), options);
+        return await this.fetchEndpoint('/v1/burns/' + encodeSegment(account, 'account'), options);
     }
 
+    // path is literals plus encodeSegment output; this method validates nothing.
     async fetchEndpoint<T>(path: string, args: any): Promise<T> {
         let response, json;
 

test/explorer-url.test.ts

@@ -89,3 +89,100 @@ describe('Explorer API URL construction', () => {
         expect(JSON.parse(calls[0].init.body)).to.deep.equal({collection_whitelist: whitelist});
     });
 });
+
+// Returns the error a call rejects with, or undefined when it resolves. The
+// guard has to fire inside the path builder, so a passing case here never
+// reaches the fetch stub.
+async function rejection(call: () => Promise<unknown>): Promise<Error | undefined> {
+    try {
+        await call();
+    } catch (e: any) {
+        return e;
+    }
+
+    return undefined;
+}
+
+describe('Explorer API path segment guard', () => {
+    it('an empty id or a dot segment rejects and sends nothing', async () => {
+        for (const id of ['', '.', '..']) {
+            const calls: FetchCall[] = [];
+            const api = mockApi(calls);
+
+            const error = await rejection(() => api.getAsset(id));
+
+            expect(error, id).to.be.instanceOf(Error);
+            expect(String(error?.message), id).to.contain('asset id');
+            expect(String(error?.message), id).to.contain('is not a valid path segment');
+            expect(String(error?.message), id).to.contain(JSON.stringify(id));
+            expect(calls.length, id).to.equal(0);
+        }
+    });
+
+    it('a missing id rejects and sends nothing', async () => {
+        const calls: FetchCall[] = [];
+        const api = mockApi(calls);
+
+        for (const value of [undefined, null]) {
+            const error = await rejection(() => api.getAsset(value as unknown as string));
+            expect(error).to.be.instanceOf(Error);
+            expect(String(error?.message)).to.equal('asset id is required');
+        }
+
+        expect(calls.length).to.equal(0);
+    });
+
+    it('a dotted name is not a dot segment and reaches the request unchanged', async () => {
+        const calls: FetchCall[] = [];
+        const api = mockApi(calls);
+
+        await api.getAccount('mycoll.wam');
+        await api.getCollection('alice.gg');
+
+        expect(calls[0].url).to.equal('https://test.api/atomicassets/v1/accounts/mycoll.wam');
+        expect(calls[1].url).to.equal('https://test.api/atomicassets/v1/collections/alice.gg');
+    });
+
+    it('every caller-supplied path segment is guarded', async () => {
+        const calls: FetchCall[] = [];
+        const api = mockApi(calls, []);
+
+        // One entry per caller-supplied segment, hand-maintained: a method
+        // added later needs its own entries here, and a listed argument
+        // position that skips the guard fails.
+        const segments: Array<[string, () => Promise<unknown>]> = [
+            ['getAsset id', () => api.getAsset('..')],
+            ['getAssetStats id', () => api.getAssetStats('..')],
+            ['getAssetLogs id', () => api.getAssetLogs('..')],
+            ['getCollection name', () => api.getCollection('..')],
+            ['getCollectionStats name', () => api.getCollectionStats('..')],
+            ['getCollectionLogs name', () => api.getCollectionLogs('..')],
+            ['getSchema collection', () => api.getSchema('..', 'myschema')],
+            ['getSchema name', () => api.getSchema('mycollection', '..')],
+            ['getSchemaStats collection', () => api.getSchemaStats('..', 'myschema')],
+            ['getSchemaStats name', () => api.getSchemaStats('mycollection', '..')],
+            ['getSchemaLogs collection', () => api.getSchemaLogs('..', 'myschema')],
+            ['getSchemaLogs name', () => api.getSchemaLogs('mycollection', '..')],
+            ['getTemplate collection', () => api.getTemplate('..', '1')],
+            ['getTemplate id', () => api.getTemplate('mycollection', '..')],
+            ['getTemplateStats collection', () => api.getTemplateStats('..', '1')],
+            ['getTemplateStats id', () => api.getTemplateStats('mycollection', '..')],
+            ['getTemplateLogs collection', () => api.getTemplateLogs('..', '1')],
+            ['getTemplateLogs id', () => api.getTemplateLogs('mycollection', '..')],
+            ['getOffer id', () => api.getOffer('..')],
+            ['getAccount account', () => api.getAccount('..')],
+            ['getAccountCollection account', () => api.getAccountCollection('..', 'mycollection')],
+            ['getAccountCollection collection', () => api.getAccountCollection('testuser2222', '..')],
+            ['getAccountBurns account', () => api.getAccountBurns('..')]
+        ];
+
+        for (const [name, call] of segments) {
+            const error = await rejection(call);
+
+            expect(error, name).to.be.instanceOf(Error);
+            expect(String(error?.message), name).to.contain('is not a valid path segment');
+        }
+
+        expect(calls.length).to.equal(0);
+    });
+});