Skip to content

Add a check for urls in loose mode. #118

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.

Sign up for GitHub

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

Closed
robert-j-webb wants to merge 1 commit into from
Closed

Add a check for urls in loose mode. #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ function sortAscending (list) {
return list.sort((a, b) => a - b);
}

/* Refer to https://drafts.csswg.org/css-syntax-3/#typedef-url-token for url token definition */
function checkForUrl (urlLike) {
console.log(urlLike);
return /[^'"\(\)\/\s]*/.test(urlLike) && !/(var|\/)/.test(urlLike);
}

module.exports = class Parser {
constructor (input, options) {
const defaults = { loose: false };
Expand Down Expand Up @@ -311,8 +317,8 @@ module.exports = class Parser {
// parens get treated as one word, if the contents aren't not a string.
if (this.current.type === 'func' && this.current.unbalanced &&
this.current.value === 'url' && this.currToken[0] !== 'string' &&
this.currToken[0] !== ')' && !this.options.loose) {

this.currToken[0] !== ')' && (checkForUrl(this.currToken[1]) || !this.options.loose)) {
Copy link

@jquense jquense May 29, 2020

Choose a reason for hiding this comment

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

should the logic here be to automatically treat url() contents as a string? technically it should follow https://drafts.csswg.org/css-syntax-3/#consume-url-token for an unquoted string, if i'm reading it right (debatable) the alg is just to consume the content unless you hit a bad char or close parens? e.g. a var() in a url shouldn't be treated as an actual var

Copy link
Owner

@shellscape shellscape May 29, 2020

Choose a reason for hiding this comment

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

agreed. I'm not sure what the spec says (I haven't looked yet) but if you try to use something like this: url(https://foo.bar/?=)bad char); in chrome and firefox, it'll mark it as invalid CSS and won't apply the rule. Even url(https://foo.bar/?=var(baz)bad char); is marked as bad after the closing var paren. I can't tell or not if either browser allow url(https://foo.bar/?=var(bad char); - there's some odd visuals there but neither will apply the rule.

Using the validator: https://jigsaw.w3.org/css-validator/#validate_by_input

fail - url(https://foo.bar/?=var)bad char);
fail - url(https://foo.bar/?=var()bad char);
fail - url(https://foo.bar/?=var(bad char);

So any extra parens when the value is not quoted will result in an error. So we should probably let the parser in place go through the unquoted tokens, and then afterward combine them all into a string. Any invalid unquoted tokens should be picked up. The only gotcha here is that CSS doesn't several unescaped characters in unquoted url params, so we'd have to identify those and run additional checks.

let nextToken = this.nextToken,
value = this.currToken[1],
start = {
Expand Down
21 changes: 14 additions & 7 deletions test/function.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('Parser → Function', () => {
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: '/gfx/img/bg.jpg', raws: { before: ' ', after: " ", quote: '\'' } },
{ type: 'word', value: ' \'/gfx/img/bg.jpg\' ' },
{ type: 'paren', value: ')' }
]
},
Expand All @@ -97,7 +97,7 @@ describe('Parser → Function', () => {
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: '/gfx/img/bg.jpg', raws: { before: ' ', after: " ", quote: '"' } },
{ type: 'word', value: ' "/gfx/img/bg.jpg" ', raws: { before: '', after: "" } },
{ type: 'paren', value: ')' }
]
},
Expand All @@ -118,11 +118,7 @@ describe('Parser → Function', () => {
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: 'http' },
{ type: 'colon', value: ':' },
{ type: 'operator', value: '/' },
{ type: 'operator', value: '/' },
{ type: 'word', value: 'domain.com/gfx/img/bg.jpg' },
{ type: 'word', value: ' http://domain.com/gfx/img/bg.jpg ' },
{ type: 'paren', value: ')' }
]
},
Expand Down Expand Up @@ -286,6 +282,17 @@ describe('Parser → Function', () => {
{ type: 'number', value: '0' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse url function with operators in it',
test: "url(http://h.c)",
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: "http://h.c" },
{ type: 'paren', value: ')' }
]
}
];

Expand Down