diff --git a/CHANGELOG.md b/CHANGELOG.md index 7944f333ca..ce0b04ba35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### 🐞 Bug fixes - ⚠️ Fix level of detail at high pitch angle by changing which tiles to load ([#3983](https://github.com/maplibre/maplibre-gl-js/issues/3983)) +- ⚠️ Fix URL parsing in `normalizeSpriteURL`, sprite URLs must be absolute. - _...Add new stuff here..._ ## 5.0.0-pre.5 @@ -34,7 +35,7 @@ ### 🐞 Bug fixes -- ⚠️ Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897)) +- ~~⚠️ Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897))~~ - ⚠️ Remove unminified prod build ([#4906](https://github.com/maplibre/maplibre-gl-js/pull/4906)) - Fix issue where raster tile source won't fetch updates following request error ([#4890](https://github.com/maplibre/maplibre-gl-js/pull/4890)) - Fix 3D models in custom layers not being properly occluded by the globe ([#4817](https://github.com/maplibre/maplibre-gl-js/issues/4817)) diff --git a/src/style/load_sprite.test.ts b/src/style/load_sprite.test.ts index b2ff73fda1..a7bdb9eda5 100644 --- a/src/style/load_sprite.test.ts +++ b/src/style/load_sprite.test.ts @@ -26,6 +26,12 @@ describe('normalizeSpriteURL', () => { ).toBe('http://www.foo.com/bar@2x.png?fresh=true'); }); + test('test relative URL', () => { + expect( + () => normalizeSpriteURL('/bar?fresh=true', '@2x', '.png') + ).toThrow(/Invalid/i); + }); + test('No Path', () => { expect( normalizeSpriteURL('http://www.foo.com?fresh=true', '@2x', '.json') @@ -66,101 +72,9 @@ describe('loadSprite', () => { const result = await promise; - expect(transform).toHaveBeenCalledTimes(1); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); - - 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('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(1); - expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1', 'Sprite'); - - 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('transform of random Sprite String', async () => { - const transform = jest.fn().mockImplementation((url, type) => { - return {url: 'http://localhost:9966/test/unit/assets/sprite1', 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('foobar', manager, 1, new AbortController()); - - server.respond(); - - const result = await promise; - - expect(transform).toHaveBeenCalledTimes(1); - expect(transform).toHaveBeenNthCalledWith(1, 'foobar', 'Sprite'); - - 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('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(1); - expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1', 'Sprite'); + expect(transform).toHaveBeenCalledTimes(2); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1.json', 'SpriteJSON'); + expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1.png', 'SpriteImage'); expect(Object.keys(result)).toHaveLength(1); expect(Object.keys(result)[0]).toBe('default'); @@ -191,9 +105,11 @@ describe('loadSprite', () => { server.respond(); const result = await promise; - expect(transform).toHaveBeenCalledTimes(2); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); - expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite2', 'Sprite'); + expect(transform).toHaveBeenCalledTimes(4); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1.json', 'SpriteJSON'); + expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1.png', 'SpriteImage'); + expect(transform).toHaveBeenNthCalledWith(3, 'http://localhost:9966/test/unit/assets/sprite2.json', 'SpriteJSON'); + expect(transform).toHaveBeenNthCalledWith(4, 'http://localhost:9966/test/unit/assets/sprite2.png', 'SpriteImage'); expect(Object.keys(result)).toHaveLength(2); expect(Object.keys(result)[0]).toBe('sprite1'); @@ -267,8 +183,9 @@ describe('loadSprite', () => { server.respond(); const result = await promise; - expect(transform).toHaveBeenCalledTimes(1); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); + expect(transform).toHaveBeenCalledTimes(2); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1@2x.json', 'SpriteJSON'); + expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1@2x.png', 'SpriteImage'); expect(Object.keys(result)).toHaveLength(1); expect(Object.keys(result)[0]).toBe('default'); diff --git a/src/style/load_sprite.ts b/src/style/load_sprite.ts index 391317442b..9482464fec 100644 --- a/src/style/load_sprite.ts +++ b/src/style/load_sprite.ts @@ -16,9 +16,14 @@ export type LoadSpriteResult = { } export function normalizeSpriteURL(url: string, format: string, extension: string): string { - const parsed = new URL(url); - parsed.pathname += `${format}${extension}`; - return parsed.toString(); + try { + const parsed = new URL(url); + parsed.pathname += `${format}${extension}`; + return parsed.toString(); + } + catch { + throw new Error(`Invalid sprite URL "${url}", must be absolute. Modify style specification directly or use TransformStyleFunction to correct the issue dynamically`) + } } export async function loadSprite( @@ -34,17 +39,11 @@ export async function loadSprite( const imagesMap: {[id: string]: Promise>} = {}; for (const {id, url} of spriteArray) { - const requestParameters = requestManager.transformRequest(url, ResourceType.Sprite); - - jsonsMap[id] = getJSON({ - ...requestParameters, - url: normalizeSpriteURL(requestParameters.url, format, '.json') - }, abortController); + const jsonRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.json'), ResourceType.SpriteJSON); + jsonsMap[id] = getJSON(jsonRequestParameters, abortController); - imagesMap[id] = ImageRequest.getImage({ - ...requestParameters, - url: normalizeSpriteURL(requestParameters.url, format, '.png') - }, abortController); + const imageRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.png'), ResourceType.SpriteImage); + imagesMap[id] = ImageRequest.getImage(imageRequestParameters, abortController); } await Promise.all([...Object.values(jsonsMap), ...Object.values(imagesMap)]); diff --git a/src/style/style.test.ts b/src/style/style.test.ts index 81be874364..6060cfc086 100644 --- a/src/style/style.test.ts +++ b/src/style/style.test.ts @@ -340,9 +340,11 @@ describe('Style#loadJSON', () => { await style.once('style.load'); - expect(transformSpy).toHaveBeenCalledTimes(1); - expect(transformSpy.mock.calls[0][0]).toBe('http://example.com/sprites/bright-v8'); - expect(transformSpy.mock.calls[0][1]).toBe('Sprite'); + expect(transformSpy).toHaveBeenCalledTimes(2); + expect(transformSpy.mock.calls[0][0]).toBe('http://example.com/sprites/bright-v8.json'); + expect(transformSpy.mock.calls[0][1]).toBe('SpriteJSON'); + expect(transformSpy.mock.calls[1][0]).toBe('http://example.com/sprites/bright-v8.png'); + expect(transformSpy.mock.calls[1][1]).toBe('SpriteImage'); }); test('emits an error on non-existant vector source layer', () => new Promise(done => { diff --git a/src/style/style.ts b/src/style/style.ts index 5c4bcb013f..4e0beef7d4 100644 --- a/src/style/style.ts +++ b/src/style/style.ts @@ -121,6 +121,7 @@ export type StyleSetterOptions = { * - when previous style carries certain 'state' that needs to be carried over to a new style gracefully; * - when a desired style is a certain combination of previous and incoming style; * - when an incoming style requires modification based on external state. + * - when an incoming style uses relative paths, which need to be converted to absolute. * * @param previous - The current style. * @param next - The next style. @@ -131,8 +132,18 @@ export type StyleSetterOptions = { * map.setStyle('https://demotiles.maplibre.org/style.json', { * transformStyle: (previousStyle, nextStyle) => ({ * ...nextStyle, + * // make relative sprite path like "../sprite" absolute + * sprite: new URL(nextStyle.sprite, "https://demotiles.maplibre.org/styles/osm-bright-gl-style/sprites/").href, + * // make relative glyphs path like "../fonts/{fontstack}/{range}.pbf" absolute + * glyphs: new URL(nextStyle.glyphs, "https://demotiles.maplibre.org/font/").href, * sources: { - * ...nextStyle.sources, + * // make relative vector url like "../../" absolute + * ...nextStyle.sources.map(source => { + * if (source.url) { + * source.url = new URL(source.url, "https://api.maptiler.com/tiles/osm-bright-gl-style/"); + * } + * return source; + * }), * // copy a source from previous style * 'osm': previousStyle.sources.osm * }, diff --git a/src/util/request_manager.ts b/src/util/request_manager.ts index f679dd0f85..bb98b284ea 100644 --- a/src/util/request_manager.ts +++ b/src/util/request_manager.ts @@ -7,7 +7,8 @@ export const enum ResourceType { Glyphs = 'Glyphs', Image = 'Image', Source = 'Source', - Sprite = 'Sprite', + SpriteImage = 'SpriteImage', + SpriteJSON = 'SpriteJSON', Style = 'Style', Tile = 'Tile', Unknown = 'Unknown',