removed url validation form normalizeSpriteURL

maplibre · Mar 31, 2024 · 5457628 · 5457628
1 parent a02fa8d
commit 5457628
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 - Added const enum for actor messages to improve readability and maintainability. In tsconfig.json, `isolatedModules` flag is set to false in favor of generated JS size. ([#3879](https://github.com/maplibre/maplibre-gl-js/issues/3879))
 
 ### 🐞 Bug fixes
+- Fix normalizeSpriteURL before transformRequest throwing an Error with reelative URLs ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897))
 - _...Add new stuff here..._
 
 ## 4.1.2

diff --git a/src/style/load_sprite.test.ts b/src/style/load_sprite.test.ts
@@ -1,12 +1,38 @@
 import fs from 'fs';
 import path from 'path';
 import {RequestManager} from '../util/request_manager';
-import {loadSprite} from './load_sprite';
+import {loadSprite, normalizeSpriteURL} from './load_sprite';
 import {type FakeServer, fakeServer} from 'nise';
 import {bufferToArrayBuffer} from '../util/test/util';
 import {ABORT_ERROR} from '../util/abort_error';
 import * as util from '../util/util';
 
+describe('normalizeSpriteURL', () => {
+    test('concantenates path, ratio, and extension for non-mapbox:// scheme', () => {
+        expect(
+            normalizeSpriteURL('http://www.foo.com/bar', '@2x', '.png')
+        ).toBe('http://www.foo.com/bar@2x.png');
+    });
+
+    test('concantenates path, ratio, and extension for file:/// scheme', () => {
+        expect(
+            normalizeSpriteURL('file:///path/to/bar', '@2x', '.png')
+        ).toBe('file:///path/to/bar@2x.png');
+    });
+
+    test('normalizes non-mapbox:// scheme when query string exists', () => {
+        expect(
+            normalizeSpriteURL('http://www.foo.com/bar?fresh=true', '@2x', '.png')
+        ).toBe('http://www.foo.com/bar@2x.png?fresh=true');
+    });
+
+    test('test relative URL', () => {
+        expect(
+            normalizeSpriteURL('/bar?fresh=true', '@2x', '.png')
+        ).toBe('/bar@2x.png?fresh=true');
+    });
+});
+
 describe('loadSprite', () => {
 
     let server: FakeServer;
@@ -56,6 +82,38 @@ describe('loadSprite', () => {
         expect(server.requests[1].url).toBe('http://localhost:9966/test/unit/assets/sprite1.png');
     });
 
+    test('transform of relative URL', async () => {
+        const transform = jest.fn().mockImplementation((url, type) => {
+            return {url: `http://localhost:9966${url}`, type};
+        });
+
+        const manager = new RequestManager(transform);
+
+        server.respondWith('GET', 'http://localhost:9966/test/unit/assets/sprite1.json', fs.readFileSync(path.join(__dirname, '../../test/unit/assets/sprite1.json')).toString());
+        server.respondWith('GET', 'http://localhost:9966/test/unit/assets/sprite1.png', bufferToArrayBuffer(fs.readFileSync(path.join(__dirname, '../../test/unit/assets/sprite1.png'))));
+
+        const promise = loadSprite('/test/unit/assets/sprite1', manager, 1, new AbortController());
+
+        server.respond();
+
+        const result = await promise;
+
+        expect(transform).toHaveBeenCalledTimes(2);
+        expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1.json', 'SpriteJSON');
+        expect(transform).toHaveBeenNthCalledWith(2, '/test/unit/assets/sprite1.png', 'SpriteImage');
+
+        expect(Object.keys(result)).toHaveLength(1);
+        expect(Object.keys(result)[0]).toBe('default');
+
+        Object.values(result['default']).forEach(styleImage => {
+            expect(styleImage.spriteData).toBeTruthy();
+            expect(styleImage.spriteData.context).toBeInstanceOf(CanvasRenderingContext2D);
+        });
+
+        expect(server.requests[0].url).toBe('http://localhost:9966/test/unit/assets/sprite1.json');
+        expect(server.requests[1].url).toBe('http://localhost:9966/test/unit/assets/sprite1.png');
+    });
+
     test('array of objects support', async () => {
         const transform = jest.fn().mockImplementation((url, type) => {
             return {url, type};

diff --git a/src/style/load_sprite.ts b/src/style/load_sprite.ts
@@ -15,6 +15,12 @@ export type LoadSpriteResult = {
     };
 }
 
+export function normalizeSpriteURL(url: string, format: string, extension: string): string {
+    const split = url.split('?');
+    split[0] += `${format}${extension}`;
+    return split.join('?');
+}
+
 export async function loadSprite(
     originalSprite: SpriteSpecification,
     requestManager: RequestManager,
@@ -28,10 +34,10 @@ export async function loadSprite(
     const imagesMap: {[id: string]: Promise<GetResourceResponse<HTMLImageElement | ImageBitmap>>} = {};
 
     for (const {id, url} of spriteArray) {
-        const jsonRequestParameters = requestManager.transformRequest(requestManager.normalizeSpriteURL(url, format, '.json'), ResourceType.SpriteJSON);
+        const jsonRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.json'), ResourceType.SpriteJSON);
         jsonsMap[id] = getJSON<SpriteJSON>(jsonRequestParameters, abortController);
 
-        const imageRequestParameters = requestManager.transformRequest(requestManager.normalizeSpriteURL(url, format, '.png'), ResourceType.SpriteImage);
+        const imageRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.png'), ResourceType.SpriteImage);
         imagesMap[id] = ImageRequest.getImage(imageRequestParameters, abortController);
     }
 

diff --git a/src/util/request_manager.test.ts b/src/util/request_manager.test.ts
diff --git a/src/util/request_manager.ts b/src/util/request_manager.ts
@@ -20,13 +20,6 @@ export const enum ResourceType {
  */
 export type RequestTransformFunction = (url: string, resourceType?: ResourceType) => RequestParameters | undefined;
 
-type UrlObject = {
-    protocol: string;
-    authority: string;
-    path: string;
-    params: Array<string>;
-};
-
 export class RequestManager {
     _transformRequestFn: RequestTransformFunction;
 
@@ -42,33 +35,8 @@ export class RequestManager {
         return {url};
     }
 
-    normalizeSpriteURL(url: string, format: string, extension: string): string {
-        const urlObject = parseUrl(url);
-        urlObject.path += `${format}${extension}`;
-        return formatUrl(urlObject);
-    }
-
     setTransformRequest(transformRequest: RequestTransformFunction) {
         this._transformRequestFn = transformRequest;
     }
 }
 
-const urlRe = /^(\w+):\/\/([^/?]*)(\/[^?]+)?\??(.+)?/;
-
-function parseUrl(url: string): UrlObject {
-    const parts = url.match(urlRe);
-    if (!parts) {
-        throw new Error(`Unable to parse URL "${url}"`);
-    }
-    return {
-        protocol: parts[1],
-        authority: parts[2],
-        path: parts[3] || '/',
-        params: parts[4] ? parts[4].split('&') : []
-    };
-}
-
-function formatUrl(obj: UrlObject): string {
-    const params = obj.params.length ? `?${obj.params.join('&')}` : '';
-    return `${obj.protocol}://${obj.authority}${obj.path}${params}`;
-}