Skip to content

Commit 88ef701

Browse files
kajkaj
committed
Limit plain css @import directives.
Fixes #139. Plain css `@import` rules is only allowed on a few condtions. Currently rsass considers any unresolved `@import` a plain css import, but in many cases it should signal an error instead. https://sass-lang.com/documentation/at-rules/import#plain-css-imports
1 parent 3c69795 commit 88ef701

File tree

24 files changed

+45
-35
lines changed

24 files changed

+45
-35
lines changed

Diff for: ‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ project adheres to
2222
### Improvements
2323

2424
* Hopefully improved relative file finding on windows (PR #137).
25+
* Plain css `@import` rules is only allowed [on a few
26+
condtions](https://sass-lang.com/documentation/at-rules/import#plain-css-imports),
27+
in other cases an error is reported (Issue #139).
2528
* Rsass can now parse (some) plain css as well as scss. Css files can
2629
be referenced in `@use` and `@import` directives, as well as in the
2730
`meta.load-css` mixin (PR #140).

Diff for: ‎src/css/string.rs

+7
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,13 @@ impl CssString {
134134
&& value.ends_with(')')
135135
&& (value.starts_with("calc(") || value.starts_with("clamp("))
136136
}
137+
/// Return true if this is a css url function call.
138+
pub(crate) fn is_css_url(&self) -> bool {
139+
let value = self.value();
140+
self.quotes() == Quotes::None
141+
&& value.ends_with(')')
142+
&& value.starts_with("url(")
143+
}
137144
/// Access the string value
138145
pub fn value(&self) -> &str {
139146
&self.value

Diff for: ‎src/file_context.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,16 @@ pub trait FileContext: Sized + std::fmt::Debug {
4242
) -> Result<Option<SourceFile>, Error> {
4343
let names: &[&dyn Fn(&str, &str) -> String] = &[
4444
// base will either be empty or end with a slash.
45-
&|base, name| format!("{}{}.scss", base, name),
46-
&|base, name| format!("{}_{}.scss", base, name),
4745
&|base, name| format!("{}{}.import.scss", base, name),
4846
&|base, name| format!("{}_{}.import.scss", base, name),
47+
&|base, name| format!("{}{}.scss", base, name),
48+
&|base, name| format!("{}_{}.scss", base, name),
49+
&|base, name| format!("{}{}/index.import.scss", base, name),
50+
&|base, name| format!("{}{}/_index.import.scss", base, name),
4951
&|base, name| format!("{}{}/index.scss", base, name),
5052
&|base, name| format!("{}{}/_index.scss", base, name),
5153
&|base, name| format!("{}{}.css", base, name),
54+
&|base, name| format!("{}_{}.css", base, name),
5255
];
5356
// Note: Should a "full stack" of bases be used here?
5457
// Or is this fine?
@@ -82,6 +85,7 @@ pub trait FileContext: Sized + std::fmt::Debug {
8285
&|base, name| format!("{}{}/index.scss", base, name),
8386
&|base, name| format!("{}{}/_index.scss", base, name),
8487
&|base, name| format!("{}{}.css", base, name),
88+
&|base, name| format!("{}_{}.css", base, name),
8589
];
8690
// Note: Should a "full stack" of bases be used here?
8791
// Or is this fine?

Diff for: ‎src/output/transform.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,18 @@ fn handle_item(
217217
scope.unlock_loading(sourcefile.path());
218218
continue 'name;
219219
}
220+
if !(x.value().starts_with("http://")
221+
|| x.value().starts_with("https://")
222+
|| x.value().starts_with("//")
223+
|| x.value().ends_with(".css")
224+
|| x.is_css_url())
225+
{
226+
return Err(Error::BadCall(
227+
"Error: Can't find stylesheet to import.".into(),
228+
pos.clone(),
229+
None,
230+
));
231+
}
220232
}
221233
let name = name.evaluate(scope.clone())?;
222234
let args = args.evaluate(scope.clone())?;
@@ -365,7 +377,7 @@ fn handle_item(
365377
})?;
366378
} else {
367379
return Err(Error::BadCall(
368-
format!("Unknown mixin {}", name),
380+
"Error: Undefined mixin.".into(),
369381
pos.clone(),
370382
None,
371383
));

Diff for: ‎tests/spec/directives/forward/error/load.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ mod test_loop {
5050
);
5151
}
5252
#[test]
53-
#[ignore] // missing error
53+
#[ignore] // wrong error
5454
fn forward_to_import() {
5555
let runner = runner().with_cwd("forward_to_import");
5656
assert_eq!(

Diff for: ‎tests/spec/directives/forward/error/member.rs

-8
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,6 @@ mod import_to_forward {
380380
);
381381
}
382382
#[test]
383-
#[ignore] // wrong error
384383
fn mixin() {
385384
let runner = runner().with_cwd("mixin");
386385
assert_eq!(
@@ -433,7 +432,6 @@ mod inaccessible {
433432
}
434433

435434
#[test]
436-
#[ignore] // wrong error
437435
fn different_separator() {
438436
let runner = runner().with_cwd("different_separator");
439437
assert_eq!(
@@ -450,7 +448,6 @@ mod inaccessible {
450448
);
451449
}
452450
#[test]
453-
#[ignore] // wrong error
454451
fn same_separator() {
455452
let runner = runner().with_cwd("same_separator");
456453
assert_eq!(
@@ -468,7 +465,6 @@ mod inaccessible {
468465
}
469466
}
470467
#[test]
471-
#[ignore] // wrong error
472468
fn mixin() {
473469
let runner = runner().with_cwd("mixin");
474470
assert_eq!(
@@ -523,7 +519,6 @@ mod inaccessible {
523519
);
524520
}
525521
#[test]
526-
#[ignore] // wrong error
527522
fn mixin() {
528523
let runner = runner().with_cwd("mixin");
529524
assert_eq!(
@@ -569,7 +564,6 @@ mod inaccessible {
569564
}
570565

571566
#[test]
572-
#[ignore] // wrong error
573567
fn mixin() {
574568
let runner = runner().with_cwd("mixin");
575569
assert_eq!(
@@ -587,7 +581,6 @@ mod inaccessible {
587581
}
588582
}
589583
#[test]
590-
#[ignore] // wrong error
591584
fn mixin() {
592585
let runner = runner().with_cwd("mixin");
593586
assert_eq!(
@@ -626,7 +619,6 @@ mod inaccessible {
626619
}
627620

628621
#[test]
629-
#[ignore] // wrong error
630622
fn mixin() {
631623
let runner = runner().with_cwd("mixin");
632624
assert_eq!(

Diff for: ‎tests/spec/directives/import/configuration.rs

+2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ mod import_twice {
115115
}
116116

117117
#[test]
118+
#[ignore] // wrong result
118119
fn no_change() {
119120
let runner = runner().with_cwd("no_change");
120121
assert_eq!(
@@ -188,6 +189,7 @@ mod indirect {
188189
);
189190
}
190191
#[test]
192+
#[ignore] // wrong result
191193
fn through_import() {
192194
let runner = runner().with_cwd("through_import");
193195
assert_eq!(

Diff for: ‎tests/spec/directives/import/error/conflict.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ mod import_only {
100100
);
101101
}
102102
#[test]
103-
#[ignore] // missing error
103+
#[ignore] // wrong error
104104
fn with_extension() {
105105
let runner = runner().with_cwd("with_extension");
106106
assert_eq!(

Diff for: ‎tests/spec/directives/import/error/member.rs

-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ mod inaccessible {
3838
);
3939
}
4040
#[test]
41-
#[ignore] // wrong error
4241
fn mixin() {
4342
let runner = runner().with_cwd("mixin");
4443
assert_eq!(

Diff for: ‎tests/spec/directives/import/error/not_found.rs

-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ fn runner() -> crate::TestRunner {
1313
}
1414

1515
#[test]
16-
#[ignore] // missing error
1716
fn directory_dot_import() {
1817
let runner = runner().with_cwd("directory_dot_import");
1918
assert_eq!(
@@ -45,7 +44,6 @@ fn no_extension() {
4544
);
4645
}
4746
#[test]
48-
#[ignore] // missing error
4947
fn parent_relative() {
5048
let runner = runner().with_cwd("parent_relative");
5149
assert_eq!(

Diff for: ‎tests/spec/directives/import/load.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ mod index {
105105
);
106106
}
107107
#[test]
108-
#[ignore] // wrong result
108+
#[ignore] // unexepected error
109109
fn sass() {
110110
let runner = runner().with_cwd("sass");
111111
assert_eq!(
@@ -152,7 +152,6 @@ mod precedence {
152152
);
153153
}
154154
#[test]
155-
#[ignore] // wrong result
156155
fn explicit_extension() {
157156
let runner = runner().with_cwd("explicit_extension");
158157
assert_eq!(
@@ -178,7 +177,6 @@ mod precedence {
178177
);
179178
}
180179
#[test]
181-
#[ignore] // wrong result
182180
fn index() {
183181
let runner = runner().with_cwd("index");
184182
assert_eq!(
@@ -205,7 +203,6 @@ mod precedence {
205203
);
206204
}
207205
#[test]
208-
#[ignore] // wrong result
209206
fn normal_before_partial() {
210207
let runner = runner().with_cwd("normal_before_partial");
211208
assert_eq!(
@@ -219,7 +216,6 @@ mod precedence {
219216
);
220217
}
221218
#[test]
222-
#[ignore] // wrong result
223219
fn partial_before_normal() {
224220
let runner = runner().with_cwd("partial_before_normal");
225221
assert_eq!(

Diff for: ‎tests/spec/directives/test_use/error/load.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ mod test_loop {
282282
);
283283
}
284284
#[test]
285-
#[ignore] // missing error
285+
#[ignore] // wrong error
286286
fn use_to_import() {
287287
let runner = runner().with_cwd("use_to_import");
288288
assert_eq!(

Diff for: ‎tests/spec/directives/test_use/error/member.rs

-2
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,6 @@ mod missing {
537537
}
538538

539539
#[test]
540-
#[ignore] // wrong error
541540
fn mixin() {
542541
let runner = runner().with_cwd("mixin");
543542
assert_eq!(
@@ -593,7 +592,6 @@ mod missing {
593592
);
594593
}
595594
#[test]
596-
#[ignore] // wrong error
597595
fn mixin() {
598596
let runner = runner().with_cwd("mixin");
599597
assert_eq!(

Diff for: ‎tests/spec/libsass_closed_issues/issue_2106/test.rs

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
109
fn test() {
1110
assert_eq!(
1211
runner().err("@import \"../does-not-exist\";\n"),

Diff for: ‎tests/spec/non_conformant/basic/t14_imports.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ fn runner() -> crate::TestRunner {
1010
}
1111

1212
#[test]
13-
#[ignore] // wrong result
13+
#[ignore] // unexepected error
1414
fn test() {
1515
assert_eq!(
1616
runner().ok("@import \"a.scss\";\n\

Diff for: ‎tests/spec/non_conformant/errors/import/miss/control_else.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/control_if.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/mixin/control_else/inside.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/mixin/control_else/outside.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/mixin/control_if/inside.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/mixin/control_if/outside.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/test_loop/each.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/test_loop/test_for.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

Diff for: ‎tests/spec/non_conformant/errors/import/miss/test_loop/test_while.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn runner() -> crate::TestRunner {
66
}
77

88
#[test]
9-
#[ignore] // missing error
9+
#[ignore] // wrong error
1010
fn test() {
1111
assert_eq!(
1212
runner().err(

0 commit comments

Comments
 (0)