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

Improvements to URL matching in guessMediaType() #1

Open
Crissov opened this issue Sep 28, 2018 · 1 comment
Open

Improvements to URL matching in guessMediaType() #1

Crissov opened this issue Sep 28, 2018 · 1 comment

Comments

@Crissov
Copy link

Crissov commented Sep 28, 2018

function guessMediaType(url) {
  const extensionMatch = url.match(/\.([^/.]+)$/);
  if (extensionMatch === null)
    return 'image';
  const extension = extensionMatch[1];
  // ...
}

That means the plugin is currently looking for a dot . followed by anything but a dot or slash before the end of the URL. If I'm not mistaken, this should yield unwanted results in these example cases:

  • http://example.mov -- match is mov, which could be a recognized video file extension and is a valid TLD
  • video.mp4?t=1m30s -- match is mp4?t=1m30s, not mp4 as intended
  • audio.mp3#chapter4 -- match is mp3#chapter4, not mp3 as intended

I'm not sure if it's usable yet, but URL.pathname should strip out host, query and hash automatically. Otherwise you could also consider them manually.

  const extensionMatch = url.match(/\/.*\.([^/.]+)(?:\?[^?]*)?(?:#[^#]*)?$/);
@eloquence
Copy link
Owner

Thanks for the report, @Crissov! I agree that the matching should be improved. I'm reluctant to add the URL dependency to simplify running the same code in Node and in the browser, and also because URL isn't supported by IE without a polyfill. But I'll do some testing with your suggested regex and may use that, or a version of that.

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

No branches or pull requests

2 participants