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

baseApiUrl w/ path & normalizeTileURL functionality #8458

Closed
sumarlidason opened this issue Jul 10, 2019 · 2 comments · Fixed by #8466
Closed

baseApiUrl w/ path & normalizeTileURL functionality #8458

sumarlidason opened this issue Jul 10, 2019 · 2 comments · Fixed by #8466

Comments

@sumarlidason
Copy link

mapbox-gl-js version: v0.54 / 1.0.0 / master

browser: chrome

I am experiencing malformed URLs generated from normalizeTileURL and makeAPIURL. This issue seems to only occur when the baseAPIURL contains a path (as opposed to just a host). I want to compare the behavior between mapbox.com & an Atlas hosted solution w/ a path of https://localhost:8080/mbx.

.com

https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7.json

is returning this tile format string:

https://a.tiles.mapbox.com/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/{z}/{x}/{y}.vector.pbf

It enters the normalizeTileURL function as:

mapbox://tiles/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/10/184/401.vector.pbf

manipulated within the function to

/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/10/184/401.vector.pbf

and finally, from makeAPIURL:

https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/10/184/401.vector.pbf

Atlas Hosted

http://localhost:8080/mbx/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7.json

is returning me a semi-accurate tile format string (the protocol is wrong):

https://localhost:8080/mbx/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/{z}/{x}/{y}.vector.pbf

It enters the normalizeTileURL function this way, however inside this function the path is manipulated, and the v4 prefix is needlessly appended:

/v4/mbx/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/10/184/401.vector.pbf

In the return statement this urlObject is passed to makeAPIURL, for a final url of

"http://localhost:8080/mbx/v4/mbx/v4/mapbox.mapbox-terrain-v2,mapbox.mapbox-streets-v7/10/184/401.vector.pbf"

You can see we got the API path prepended again.

Seeking Guidance

I don't understand the rewriting of the url format provided by the /v4 request. Additionally it looks like we are losing the subdomaining browser hack for increased parallelized requests?

sumarlidason pushed a commit that referenced this issue Jul 11, 2019
@sumarlidason
Copy link
Author

Executing Test Case

git fetch
git checkout 8458_debugNormalizeTileURL
yarn test-unit util/mapbox.test.js

This test replicates the issue I am seeing above:
image

sumarlidason pushed a commit that referenced this issue Jul 11, 2019
- adds regex to trim tileURL prefix to /v4/...
- adds filter to makeAPIURL preventing duplication of param 'access_token'
- reorders params on test case -> sku,access_token

closes #8458
@sumarlidason
Copy link
Author

Possible fix w/ 37ab6b7

  1. added regex to trim the tileURL:
    const imageExtensionRe = /(\.(png|jpg)\d*)(?=$)/;
    const tileURLAPIPrefixRe = /^.+\/v4\//;
    const normalizeTileURL = function(tileURL: string, sourceURL?: ?string, tileSize?: ?number, skuToken?: string, customAccessToken?: ?string): string {
    if (!sourceURL || !isMapboxURL(sourceURL)) return tileURL;
    const urlObject = parseUrl(tileURL);
    // The v4 mapbox tile API supports 512x512 image tiles only when @2x
    // is appended to the tile URL. If `tileSize: 512` is specified for
    // a Mapbox raster source force the @2x suffix even if a non hidpi device.
    const suffix = browser.devicePixelRatio >= 2 || tileSize === 512 ? '@2x' : '';
    const extension = webpSupported.supported ? '.webp' : '$1';
    urlObject.path = urlObject.path.replace(imageExtensionRe, `${suffix}${extension}`);
    urlObject.path = urlObject.path.replace(tileURLAPIPrefixRe, '/');
    urlObject.path = `/v4${urlObject.path}`;
    if (config.REQUIRE_ACCESS_TOKEN && (config.ACCESS_TOKEN || customAccessToken) && skuToken) {
    urlObject.params.push(`sku=${skuToken}`);
    }
    return makeAPIURL(urlObject, customAccessToken);
    };
  2. made makeAPIURL a bit smarter about params, removing any existing access_token that was provided and replacing with the mapbox-gl-js configured token.
    urlObject.params = urlObject.params.filter((d) => !d.includes('access_token'));
    urlObject.params.push(`access_token=${accessToken}`);
    return formatUrl(urlObject);

I am not certain this is the best solution -

sumarlidason pushed a commit that referenced this issue Jul 11, 2019
- adds regex to trim tileURL prefix to /v4/...
- adds filter to makeAPIURL preventing duplication of param 'access_token'
- reorders params on test case -> sku,access_token

closes #8458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants