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

Enable URLs with querystring #117

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rochoa
Copy link

@rochoa rochoa commented Nov 3, 2014

No description provided.

@springmeyer
Copy link
Member

Great, thanks. Would appreciate a quick summary of what this fixes specifically.

@rochoa
Copy link
Author

rochoa commented Nov 3, 2014

Sorry, I should have been more clear about what the issue was or create an issue before.

Basically this allows to use URLs like: http://i.imgur.com/yvRISk8.png?foo=bar.

@rochoa
Copy link
Author

rochoa commented Nov 3, 2014

Check:

> var OLD_REGEX = /[-a-zA-Z0-9@:%_\+.~#?&//=]{2,256}\.[a-z]{2,4}\b(\/[-a-zA-Z0-9@:%_\+.~#?&//=]*)?/gi
undefined
> var NEW_REGEX = /[-a-zA-Z0-9@:%_\+.~#?&//=]{2,256}\.[a-z]{2,4}\b(\/[-a-zA-Z0-9@:%_\+.~#?&//=]*)/gi
undefined
> 'http://i.imgur.com/yvRISk8.png?foo=bar'.match(OLD_REGEX)
[ 'http://i.imgur.com/yvRISk8.png' ]
> 'http://i.imgur.com/yvRISk8.png?foo=bar'.match(NEW_REGEX)
[ 'http://i.imgur.com/yvRISk8.png?foo=bar' ]

I don't know if it could be better to move the extract urls from carto to it's own method so we can unit test it but at the same time I don't like to expose that method in the module/public api. Maybe move it to utils.js?

@jchamberlain
Copy link

I know this is old, but I'd love to see this or something equivalent go in. In our app, all user-uploaded files are stored in an S3 bucket that's not publicly accessible. Rather than making an exception for images intended for maps, we inject presigned URLs into the CartoCSS for marker files, etc. Problem is, presigned URLs have query strings which are currently being stripped.

@rochoa
Copy link
Author

rochoa commented Dec 9, 2016

I just updated the branch to solve conflicts on CHANGELOG file.

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

Successfully merging this pull request may close these issues.

3 participants