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

Replace request with axios #1043

Merged
merged 7 commits into from
Nov 14, 2023
Merged

Replace request with axios #1043

merged 7 commits into from
Nov 14, 2023

Conversation

acalcutt
Copy link
Collaborator

@acalcutt acalcutt commented Nov 2, 2023

This is a cherry pick of the PR acalcutt#284 submitted by @mjj203 in my old fork. I have updated it a little to fix some things changed in the current release.

This replaces request, which is unmaintained and has serveral vulnerabilies, with axios.

Fixes #1006

Copy link
Contributor

@mnutt mnutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Great to see us replacing request.

axios is a good batteries-included http library. In my fork I used undici, which (as of node 16) will become the official node.js request library and uses web-standard fetch, but I think axios is also a good choice.

The code looks fine too. Down the road we might have an opportunity to refactor the maplibre data loading; maybe to something like:

const renderer = new mbgl.Map({
  mode: mode,
  ratio: ratio,
  request: (req, callback) => {
    requestMap(req.url)
      .then((data) => callback(null, data))
      .catch((err) => callback(err, null));
});

function requestMap(url) {
  const protocol = url.split(":")[0];

  if (protocol === "sprites") {
    return requestSprites(url);
  } else if (protocol === "fonts") {
    return requestFonts(url);
  } else if (protocol === "mbtiles") {
    return requestMbtiles(url, dataDecoratorFunc);
  } else if (protocol === "http" || protocol === "https") {
    return requestUrl(url);
  } else {
    return Promise.reject(new Error(`Unkown protocol: ${protocol}`));
  }
}

This way we have the callback->promise transition fairly early on and can just deal in promises.

Signed-off-by: Andrew Calcutt <[email protected]>
@acalcutt acalcutt merged commit 36f63de into maptiler:master Nov 14, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Replace 'request'
2 participants