Skip to content

fix: ban import assertions during publishing #427

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

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 73 additions & 0 deletions api/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::collections::HashSet;
use std::sync::Arc;

use deno_ast::swc::common::comments::CommentKind;
use deno_ast::swc::common::Span;
use deno_ast::LineAndColumnDisplay;
use deno_ast::MediaType;
use deno_ast::ModuleSpecifier;
Expand Down Expand Up @@ -727,6 +728,54 @@ fn check_for_banned_syntax(
continue;
}
},
ast::ModuleDecl::Import(n) => {
if let Some(with) = &n.with {
let range =
Span::new(n.src.span.hi(), with.span.lo(), n.src.span.ctxt)
.range();
let keyword = parsed_source.text_info().range_text(&range);
if keyword.contains("assert") {
let (line, column) = line_col(&with.span.range());
return Err(PublishError::BannedImportAssertion {
specifier: parsed_source.specifier().to_string(),
line,
column,
});
}
}
}
ast::ModuleDecl::ExportNamed(n) => {
if let Some(with) = &n.with {
let src = n.src.as_ref().unwrap();
let range =
Span::new(src.span.hi(), with.span.lo(), src.span.ctxt).range();
let keyword = parsed_source.text_info().range_text(&range);
if keyword.contains("assert") {
let (line, column) = line_col(&with.span.range());
return Err(PublishError::BannedImportAssertion {
specifier: parsed_source.specifier().to_string(),
line,
column,
});
}
}
}
ast::ModuleDecl::ExportAll(n) => {
if let Some(with) = &n.with {
let range =
Span::new(n.src.span.hi(), with.span.lo(), n.src.span.ctxt)
.range();
let keyword = parsed_source.text_info().range_text(&range);
if keyword.contains("assert") {
let (line, column) = line_col(&with.span.range());
return Err(PublishError::BannedImportAssertion {
specifier: parsed_source.specifier().to_string(),
line,
column,
});
}
}
}
_ => continue,
},
ast::ModuleItem::Stmt(n) => match n {
Expand Down Expand Up @@ -920,5 +969,29 @@ mod tests {

let x = parse("import express = React.foo;");
assert!(super::check_for_banned_syntax(&x).is_ok());

let x = parse("import './data.json' assert { type: 'json' }");
let err = super::check_for_banned_syntax(&x).unwrap_err();
assert!(
matches!(err, super::PublishError::BannedImportAssertion { .. }),
"{err:?}",
);

let x = parse("export { a } from './data.json' assert { type: 'json' }");
let err = super::check_for_banned_syntax(&x).unwrap_err();
assert!(
matches!(err, super::PublishError::BannedImportAssertion { .. }),
"{err:?}",
);

let x = parse("export * from './data.json' assert { type: 'json' }");
let err = super::check_for_banned_syntax(&x).unwrap_err();
assert!(
matches!(err, super::PublishError::BannedImportAssertion { .. }),
"{err:?}",
);

let x = parse("export * from './data.json' with { type: 'json' }");
assert!(super::check_for_banned_syntax(&x).is_ok(), "{err:?}",);
}
}
35 changes: 35 additions & 0 deletions api/src/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,41 @@ pub mod tests {
assert_eq!(error.code, "invalidPath");
}

#[tokio::test]
async fn import_assertions() {
let t = TestSetup::new().await;

let bytes = create_mock_tarball("import_assertions");
let task = process_tarball_setup2(
&t,
bytes,
&PackageName::try_from("foo").unwrap(),
&Version::try_from("1.2.3").unwrap(),
false,
)
.await;
assert_eq!(task.status, PublishingTaskStatus::Failure, "{task:#?}");
let error = task.error.unwrap();
assert_eq!(error.code, "bannedImportAssertion");
assert_eq!(error.message, "import assertions are not allowed, use import attributes instead (replace 'assert' with 'with') file:///mod.ts:1:29");
}

#[tokio::test]
async fn import_attributes() {
let t = TestSetup::new().await;

let bytes = create_mock_tarball("import_attributes");
let task = process_tarball_setup2(
&t,
bytes,
&PackageName::try_from("foo").unwrap(),
&Version::try_from("1.2.3").unwrap(),
false,
)
.await;
assert_eq!(task.status, PublishingTaskStatus::Success, "{task:#?}");
}

#[tokio::test]
async fn jsr_jsonc() {
let t = TestSetup::new().await;
Expand Down
14 changes: 12 additions & 2 deletions api/src/tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ pub enum PublishError {
#[error("invalid external import to '{specifier}', only 'jsr:', 'npm:', 'data:' and 'node:' imports are allowed ({info})")]
InvalidExternalImport { specifier: String, info: String },

#[error("Modifying global types is not allowed {specifier}:{line}:{column}")]
#[error("modifying global types is not allowed {specifier}:{line}:{column}")]
GlobalTypeAugmentation {
specifier: String,
line: usize,
Expand All @@ -499,13 +499,20 @@ pub enum PublishError {
column: usize,
},

#[error("Triple slash directives that modify globals (for example, '/// <reference no-default-lib=\"true\" />' or '/// <reference lib=\"dom\" />') are not allowed. Instead instruct the user of your package to specify these directives. {specifier}:{line}:{column}")]
#[error("triple slash directives that modify globals (for example, '/// <reference no-default-lib=\"true\" />' or '/// <reference lib=\"dom\" />') are not allowed. Instead instruct the user of your package to specify these directives. {specifier}:{line}:{column}")]
BannedTripleSlashDirectives {
specifier: String,
line: usize,
column: usize,
},

#[error("import assertions are not allowed, use import attributes instead (replace 'assert' with 'with') {specifier}:{line}:{column}")]
BannedImportAssertion {
specifier: String,
line: usize,
column: usize,
},

#[error(
"file at path '{path}' too large, max size is {max_size}, got {size}"
)]
Expand Down Expand Up @@ -614,6 +621,9 @@ impl PublishError {
PublishError::BannedTripleSlashDirectives { .. } => {
Some("bannedTripleSlashDirectives")
}
PublishError::BannedImportAssertion { .. } => {
Some("bannedImportAssertion")
}
PublishError::InvalidExternalImport { .. } => {
Some("invalidExternalImport")
}
Expand Down
1 change: 1 addition & 0 deletions api/testdata/tarballs/import_assertions/data.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"abc"
5 changes: 5 additions & 0 deletions api/testdata/tarballs/import_assertions/jsr.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "@scope/foo",
"version": "1.2.3",
"exports": "./mod.ts"
}
1 change: 1 addition & 0 deletions api/testdata/tarballs/import_assertions/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "./data.json" assert { type: "json" };
1 change: 1 addition & 0 deletions api/testdata/tarballs/import_attributes/data.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"abc"
5 changes: 5 additions & 0 deletions api/testdata/tarballs/import_attributes/jsr.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "@scope/foo",
"version": "1.2.3",
"exports": "./mod.ts"
}
1 change: 1 addition & 0 deletions api/testdata/tarballs/import_attributes/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "./data.json" with { type: "json" };
11 changes: 11 additions & 0 deletions frontend/docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@ allowed by JSR. JSR only allows triple slash directives that are

You can fix this error by removing the triple slash directive from your source.

### `bannedImportAssertion`

The package being published contains the legacy "import assertions" syntax,
which is not allowed by JSR. JSR only allows the new "import attributes" syntax.

`import "./data.json" assert { type: "json" };` is not allowed.
`import "./data.json" with { type: "json" };` is allowed.

You can fix this error by updating the import assertion to an import attribute,
by replacing `assert` with `with`.

### `fileTooLarge`

The package being published contains a file that is too large. JSR only allows
Expand Down