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

Permit multiline in URL #1096

Closed
am11 opened this issue Apr 11, 2015 · 17 comments · Fixed by #1196
Closed

Permit multiline in URL #1096

am11 opened this issue Apr 11, 2015 · 17 comments · Fixed by #1196

Comments

@am11
Copy link
Contributor

am11 commented Apr 11, 2015

Test:

@import url("a\
b");

LibSass master (0dd6543) errors:

URI is missing ')'

LibSass 3.1 outputs:

@import url("a\
b");

RubySass v3.4.12 outputs:

@import url("ab");

Related test:

// without quotes
@import url(a\
b);

RubySass throws the following error:

Invalid CSS after "@import url(a": expected ")", was ""

LibSass master (0dd6543) errors:

URI is missing ')'

LibSass 3.1 outputs:

@import url(a\
b);
@xzyfer
Copy link
Contributor

xzyfer commented Apr 25, 2015

Specs added sass/sass-spec#345

mgreter added a commit to mgreter/libsass that referenced this issue May 10, 2015
@mgreter mgreter added this to the 3.2.4 milestone May 10, 2015
@mgreter mgreter self-assigned this May 10, 2015
mgreter added a commit to mgreter/libsass that referenced this issue May 11, 2015
mgreter added a commit to mgreter/libsass that referenced this issue May 11, 2015
mgreter added a commit to mgreter/libsass that referenced this issue May 12, 2015
@am11
Copy link
Contributor Author

am11 commented May 12, 2015

@mgreter, there is still one crazy case which is inconsistent (perhaps not worth fixing?).

(added periods to highlight spaces)

@import url("a.. .\.. .
 . . b");

Ruby Sass:

@import url("a.. ... .\a  . . b");

Current libsass master (28ee088):

@import url("a.. ... .
\a  . . b");

Not sure about logic behind ruby-sass implementation. I was expecting something like this:

@import url("a.. ... .\  . . b");

@xzyfer xzyfer reopened this May 13, 2015
@xzyfer
Copy link
Contributor

xzyfer commented May 13, 2015

This still fails the spec.

@xzyfer xzyfer removed this from the 3.2.4 milestone May 13, 2015
@xzyfer
Copy link
Contributor

xzyfer commented May 13, 2015

Apologies, it looks like my local sass-spec is having issues.

@xzyfer
Copy link
Contributor

xzyfer commented May 13, 2015

This fix was a false positive due to sass/sass-spec#380. The spec still fails.

@xzyfer xzyfer reopened this May 13, 2015
@xzyfer xzyfer removed this from the 3.2.4 milestone May 13, 2015
@xzyfer xzyfer removed this from the 3.3.1 milestone Oct 26, 2015
@xzyfer xzyfer modified the milestones: 3.3.2, 3.3.3 Nov 4, 2015
xzyfer added a commit to xzyfer/libsass that referenced this issue Dec 28, 2015
This is a continuation of the previous work done to bring our
`url()` parsing up to scratch with Ruby Sass. With this update
we can remove the trasnitional legacy `url()` parsing logic.

Fixes sass#1096
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Dec 28, 2015
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Dec 28, 2015
xzyfer added a commit to xzyfer/libsass that referenced this issue Dec 28, 2015
This is a continuation of the previous work done to bring our
`url()` parsing up to scratch with Ruby Sass. With this update
we can remove the transitional legacy `url()` parsing logic.

Fixes sass#1096
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Dec 28, 2015
@am11
Copy link
Contributor Author

am11 commented Dec 28, 2015

@xzyfer,

quickly tested current master with the following input:

/* url-test1: without quotes */
@import url(a
b);

/* url-test1: with quotes */
@import url("a
b");

/* url-test2: with quotes */
@import url("a   \d \\d \\\d \\\\d
    c \t \n \\t \\n \\\t \\\n \\\\t \\\\n");

/* url-test2: without quotes */
@import url(a   \d \\d \\\d \\\\d
    c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

LibSass:

/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
\a b"); url("a
/* url-test2: with quotes */
\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

RubySass:

/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
@import url("a\a b");
/* url-test2: with quotes */
@import url("a   
\\d \\
\\\\d\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

IMO, url-test1: with quotes is more important than rest of them.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 29, 2015

Interesting. I just tested with 0f929a5 and got identical output (minus deprecation warnings)

/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
@import url("a\a b");
/* url-test2: with quotes */
\\\\d\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

@am11
Copy link
Contributor Author

am11 commented Dec 29, 2015

Yup, I tested on both Mac and Windows, the result is identical to yours. url-test1 in Windows is inverted from the middle. url-test2 in both are differently incorrect. I used libsass a508a7c and sassc sass/sassc@94838a9:

~/Documents/projects/libsass/sassc/bin/sassc ~/Documents/temp/blah.scss

@xzyfer
Copy link
Contributor

xzyfer commented Dec 29, 2015

@am11 sorry I'm not following your previous comment. Please be more explicitly.

Can you please post the output you're seeing on Windows, and how it differs from Mac.

Interestingly I'm unable to get the Ruby Sass output you've provided locally (on Mac), but I do see it on sassmeister.

@am11
Copy link
Contributor Author

am11 commented Dec 29, 2015

I am running Mac and Windows 10 via Parallels. I am using sassc.

Mac (sassc running on Terminal):

~/Documents/projects/libsass/sassc/bin/sassc ~/Documents/temp/blah.scss
/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
@import url("a\a b");
/* url-test2: with quotes */
\\\\d\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

Windows (sassc running on PowerShell):

~/Documents/projects/libsass/sassc/bin/sassc ~/Documents/temp/blah.scss
/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
\a b"); url("a
/* url-test2: with quotes */
\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

Line below /* url-test1: with quotes */ on Windows is inverted from the middle.

Line below /* url-test2: with quotes */ on both Mac and Windows are incorrect but different from each other.

@am11
Copy link
Contributor Author

am11 commented Dec 29, 2015

@xzyfer, sorry I was looking at the CLI output which was broken in both PowerShell and Terminal in different ways.

I just piped to a file and saw the output in editor.

scssc  blah.scss > blah.css

Mac (TextEdit)

/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
@import url("a\a b");
/* url-test2: with quotes */
@import url("a   
\\d \\
\\\\d\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

Windows (Visual Studio and then HexEdit to check what's going on):

/* url-test1: without quotes */
@import url(a b);
/* url-test1: with quotes */
@import url("a
\a b");
/* url-test2: with quotes */
@import url("a   
\\d \\
\\\\d
\a     c t n \\t \\n \\t \\n \\\\t \\\\n");
/* url-test2: without quotes */
@import url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n);

Note that in Windows output, the extra line before each \a is \r (HEX 0D). I haven't tested whether it is the raw result of compiler or is it shell redirection inserting the carriage return there. Whichever it is, this issue is fixed. Thank you! 🏆 👍

@am11
Copy link
Contributor Author

am11 commented Dec 30, 2015

@xzyfer, I have confirmed that \r (hex: 0D) is added before both \a occurrences, by placing printf("size >>> %lu\n", strlen(output_string)); before: https://github.com/sass/sassc/blob/94838a9/sassc.c#L46.

When compiling this input sample on Mac it prints: size >>> 308 while on Windows: size >>> 310.

Any ideas where this \r it is coming from?

@xzyfer
Copy link
Contributor

xzyfer commented Dec 30, 2015

@am11 nothing comes to mind. Also I don't think this patch affected quoted
imports. Can you confirm whether this issue existed before this patch?
On 30 Dec 2015 8:23 pm, "Adeel Mujahid" notifications@github.com wrote:

@xzyfer https://github.com/xzyfer, I have confirmed that \r (hex: 0D)
is added before both \a occurrences, by placing printf("size >>> %lu\n",
strlen(output_string)); before:
https://github.com/sass/sassc/blob/94838a9/sassc.c#L46.

When compiling this input sample
#1096 (comment) on
Mac it prints: size >>> 308 while on Windows: size >>> 310.

Any ideas where this \r it is coming from?


Reply to this email directly or view it on GitHub
#1096 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Dec 30, 2015

Also if you include debugger.cpp in context.cpp and add

debug_ast(root);

on line 657 the AST tree will be dumped to the console just before the output phase begins.

@am11
Copy link
Contributor Author

am11 commented Dec 30, 2015

With debugger, on Mac:

####################################################################
  Block 0x7fddcbc1d410 (0@[0:0]-[16:0]) [root] 0
  Comment 0x7fddcbc1c1a0 (0@[0:0]-[0:31]) 0 <>
  // String_Quoted 0x7fddcbc07460 (0@[0:0]-[0:31]) [/* url-test1: without quotes */] [delayed] <>
  Import 0x7fddcbc1c230 (0@[1:0]-[1:7]) 0
 @ String_Quoted 0x7fddcbc1c570 (0@[1:8]-[1:12]) [url(a b)] < >
 Comment 0x7fddcbc1c750 (0@[4:0]-[4:28]) 0 <\n\n>
 // String_Quoted 0x7fddcbc07ad0 (0@[4:0]-[4:28]) [/* url-test1: with quotes */] [delayed] <\n\n>
 Import 0x7fddcbc1c690 (0@[5:0]-[5:7]) 0
@ String_Quoted 0x7fddcbc1ca10 (0@[5:8]-[5:12]) [url("a\a b")] < >
 Comment 0x7fddcbc1cb60 (0@[8:0]-[8:28]) 0 <\n\n>
 // String_Quoted 0x7fddcbc08050 (0@[8:0]-[8:28]) [/* url-test2: with quotes */] [delayed] <\n\n>
 Import 0x7fddcbc1caa0 (0@[9:0]-[9:7]) 0
@ String_Quoted 0x7fddcbc1cdd0 (0@[9:8]-[9:12]) [url("a   \r\\d \\\r\\\\d\a     c t n \\t \\n \\t \\n \\\\t \\\\n")] < >
 Comment 0x7fddcbc1ceb0 (0@[12:0]-[12:31]) 0 <\n  \n>
 // String_Quoted 0x7fddcbc08570 (0@[12:0]-[12:31]) [/* url-test2: without quotes */] [delayed] <\n  \n>
 Import 0x7fddcbc1cf40 (0@[13:0]-[13:7]) 0
@ String_Quoted 0x7fddcbc1d2e0 (0@[13:8]-[13:12]) [url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n)] < >
####################################################################

Windows:

####################################################################
Block 00B236C0 (0@[0:0]-[15:1]) [root] 0
 Comment 00B1F080 (0@[0:0]-[0:31]) 0 <>
 // String_Quoted 00AEA178 (0@[0:0]-[0:31]) [/* url-test1: without quotes */] [delayed] <>
 Import 00B06B68 (0@[1:0]-[1:7]) 0
@ String_Quoted 00B06DA8 (0@[1:8]-[1:12]) [url(a b)] < >
 Comment 00B1F0F8 (0@[4:0]-[4:28]) 0 <\r\n\r\n>
 // String_Quoted 00AEC078 (0@[4:0]-[4:28]) [/* url-test1: with quotes */] [delayed] <\r\n\r\n>
 Import 00B06FE8 (0@[5:0]-[5:7]) 0
@ String_Quoted 00B07228 (0@[5:8]-[5:12]) [url("a\r\a b")] < >
 Comment 00B1EEA0 (0@[8:0]-[8:28]) 0 <\r\n\r\n>
 // String_Quoted 00AEC1A8 (0@[8:0]-[8:28]) [/* url-test2: with quotes */] [delayed] <\r\n\r\n>
 Import 00B273B0 (0@[9:0]-[9:7]) 0
@ String_Quoted 00B27680 (0@[9:8]-[9:12]) [url("a   \r\\d \\\r\\\\d\r\a     c t n \\t \\n \\t \\n \\\\t \\\\n")] < >
 Comment 00B1F170 (0@[12:0]-[12:31]) 0 <\r\n  \r\n>
 // String_Quoted 00B06808 (0@[12:0]-[12:31]) [/* url-test2: without quotes */] [delayed] <\r\n  \r\n>
 Import 00B28400 (0@[13:0]-[13:7]) 0
@ String_Quoted 00B27F80 (0@[13:8]-[13:12]) [url(a \d \\d \\\d \\\\d c \t \n \\t \\n \\\t \\\n \\\\t \\\\n)] < >
####################################################################

Line 10 indicates

Mac: @ String_Quoted 0x7fddcbc1ca10 (0@[5:8]-[5:12]) [url("a\a b")] < >
Win: @ String_Quoted 00B07228 (0@[5:8]-[5:12]) [url("a\r\a b")] < >

You are right, this issue was present before your fix: 2ecaa66. Note that before 15913ab, my sample was throwing SIGSEGV on both OSes, so I tested just one case: url-test1: with quotes. 😄

@xzyfer
Copy link
Contributor

xzyfer commented Dec 30, 2015

Interesting. Looks like it's worth creating an issue for. Not exactly sure how I'll debug it though haha

You can debug_ast(root) after each previous assignment of root to see if it's the parser, or eval step that introduces the \r.

am11 added a commit to am11/libsass that referenced this issue Dec 30, 2015
Input:

```scss
@import url("a
b");
```

Output on Windows:

```css
"@import url(\"a\r\a b\")";
```

Output on Unix (alinged with Ruby Sass, hence the expected output):

```css
"@import url(\"a\a b\")";
```

See: sass#1096 (comment)
for details.
am11 added a commit to am11/libsass that referenced this issue Dec 30, 2015
Input:

```scss
@import url("a
b");
```

Output on Windows:

```
"@import url(\"a\r\a b\");"
```

Output on Unix (alinged with Ruby Sass, hence the expected output):

```
"@import url(\"a\a b\");"
```

See: sass#1096 (comment)
for details.
am11 added a commit to am11/libsass that referenced this issue Dec 30, 2015
Input:

```scss
@import url("a
b");
```

Output on Windows:

```
"@import url(\"a\r\a b\");"
```

Output on Unix (alinged with Ruby Sass, hence the expected output):

```
"@import url(\"a\a b\");"
```

See: sass#1096 (comment)
for details.
am11 added a commit to am11/libsass that referenced this issue Dec 30, 2015
Input:

```scss
@import url("a
b");
```

Output on Windows:

```
"@import url(\"a\r\a b\");"
```

Output on Unix (alinged with Ruby Sass, hence the expected output):

```
"@import url(\"a\a b\");"
```

See: sass#1096 (comment)
for details.
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Jan 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants