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

#361: Fix for quoted data URIs getting prepended with path #432

Merged
merged 1 commit into from
Oct 23, 2011

Conversation

asolove
Copy link
Contributor

@asolove asolove commented Oct 21, 2011

In the browser environment, quoted data URIs are prepended with the import path.

The issue superficially appeared to be fixed because the tests pass, but that was because the codepath with the bug is only run in the browser environment.

This ticket fixes the problem by correctly pattern-matching for valid data URIs, which do not require a slash.

cloudhead pushed a commit that referenced this pull request Oct 23, 2011
#361: Fix for quoted data URIs getting prepended with path
@cloudhead cloudhead merged commit 1751f13 into less:master Oct 23, 2011
@asolove
Copy link
Contributor Author

asolove commented Oct 24, 2011

This actually created a much larger browser-only bug, where all strings are treated as absolute URLs. This happened because I removed the slash after the optional group, so the regex now always matches all strings:

/^(?:https?:\/\/|file:\/\/|data:)?/ // matches a string starting with anything

The question is what to do in the case where the URL starts with a slash. If urls starting with a slash are treated as absolute, we can simply change the regex to:

/^(?:https?:\/\/|file:\/\/|data:|\/)/ // matches a string with http(s), file, data, or a slash

But the line inside the conditional, which only runs if the regex doesn't match, (and therefore the url is being treated as relative) seems to expect that urls starting with slash might come to it:

val.value = paths[0] + (val.value.charAt(0) === '/' ? val.value.slice(1) : val.value);

In which case the regex should be changed to non-optional but not include the slash option:

/^(?:https?:\/\/|file:\/\/|data:)/ // matches a string with http(s), file, or data only.

stefanklug pushed a commit to stefanklug/carto that referenced this pull request Jan 27, 2019
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.

2 participants