From 65327e0e7e7e44c0734e9af5058a8f3f017f2bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 4 Sep 2022 23:19:20 +0300 Subject: [PATCH] Disable cyclical module imports (#6477) --- crates/nu-parser/src/errors.rs | 5 + crates/nu-parser/src/parse_keywords.rs | 42 ++++++++- crates/nu-protocol/src/engine/engine_state.rs | 3 + tests/modules/mod.rs | 92 +++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 17782684d5..6e158d7b83 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -120,6 +120,10 @@ pub enum ParseError { )] ModuleNotFound(#[label = "module not found"] Span), + #[error("Cyclical module import.")] + #[diagnostic(code(nu::parser::cyclical_module_import), url(docsrs), help("{0}"))] + CyclicalModuleImport(String, #[label = "detected cyclical module import"] Span), + #[error("Active overlay not found.")] #[diagnostic(code(nu::parser::active_overlay_not_found), url(docsrs))] ActiveOverlayNotFound(#[label = "not an active overlay"] Span), @@ -341,6 +345,7 @@ impl ParseError { ParseError::VariableNotFound(s) => *s, ParseError::VariableNotValid(s) => *s, ParseError::ModuleNotFound(s) => *s, + ParseError::CyclicalModuleImport(_, s) => *s, ParseError::ModuleOrOverlayNotFound(s) => *s, ParseError::ActiveOverlayNotFound(s) => *s, ParseError::OverlayPrefixMismatch(_, _, s) => *s, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index a84c2bc67f..0069e7e285 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1660,8 +1660,8 @@ pub fn parse_use( if let Some(module_id) = working_set.find_module(&import_pattern.head.name) { (import_pattern, working_set.get_module(module_id).clone()) } else { - // TODO: Do not close over when loading module from file? // It could be a file + // TODO: Do not close over when loading module from file? let (module_filename, err) = unescape_unquote_string(&import_pattern.head.name, import_pattern.head.span); @@ -1670,6 +1670,37 @@ pub fn parse_use( if let Some(module_path) = find_in_dirs(&module_filename, working_set, &cwd, LIB_DIRS_ENV) { + if let Some(i) = working_set + .parsed_module_files + .iter() + .rposition(|p| p == &module_path) + { + let mut files: Vec = working_set + .parsed_module_files + .split_off(i) + .iter() + .map(|p| p.to_string_lossy().to_string()) + .collect(); + + files.push(module_path.to_string_lossy().to_string()); + + let msg = files.join("\nuses "); + + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: call_span, + ty: Type::Any, + custom_completion: None, + }]), + vec![], + Some(ParseError::CyclicalModuleImport( + msg, + import_pattern.head.span, + )), + ); + } + let module_name = if let Some(stem) = module_path.file_stem() { stem.to_string_lossy().to_string() } else { @@ -1690,7 +1721,7 @@ pub fn parse_use( working_set.add_file(module_filename, &contents); let span_end = working_set.next_span_start(); - // Change currently parsed directory + // Change the currently parsed directory let prev_currently_parsed_cwd = if let Some(parent) = module_path.parent() { let prev = working_set.currently_parsed_cwd.clone(); @@ -1701,6 +1732,10 @@ pub fn parse_use( working_set.currently_parsed_cwd.clone() }; + // Add the file to the stack of parsed module files + working_set.parsed_module_files.push(module_path); + + // Parse the module let (block, module, err) = parse_module_block( working_set, Span::new(span_start, span_end), @@ -1708,6 +1743,9 @@ pub fn parse_use( ); error = error.or(err); + // Remove the file from the stack of parsed module files + working_set.parsed_module_files.pop(); + // Restore the currently parsed directory back working_set.currently_parsed_cwd = prev_currently_parsed_cwd; diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 6fbd0e528f..5855896c86 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -777,6 +777,8 @@ pub struct StateWorkingSet<'a> { pub type_scope: TypeScope, /// Current working directory relative to the file being parsed right now pub currently_parsed_cwd: Option, + /// All previously parsed module files. Used to protect against circular imports. + pub parsed_module_files: Vec, } /// A temporary placeholder for expression types. It is used to keep track of the input types @@ -952,6 +954,7 @@ impl<'a> StateWorkingSet<'a> { external_commands: vec![], type_scope: TypeScope::default(), currently_parsed_cwd: None, + parsed_module_files: vec![], } } diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs index 221bbf0106..3bb7a6cee2 100644 --- a/tests/modules/mod.rs +++ b/tests/modules/mod.rs @@ -341,3 +341,95 @@ fn module_import_env_2() { assert_eq!(actual.out, "foo"); }) } + +#[test] +fn module_cyclical_imports_0() { + Playground::setup("module_cyclical_imports_0", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContentToBeTrimmed( + "spam.nu", + r#" + use eggs.nu + "#, + )]); + + let inp = &[r#"module eggs { use spam.nu }"#]; + + let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; "))); + + assert!(actual.err.contains("module not found")); + }) +} + +#[test] +fn module_cyclical_imports_1() { + Playground::setup("module_cyclical_imports_1", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContentToBeTrimmed( + "spam.nu", + r#" + use spam.nu + "#, + )]); + + let inp = &[r#"use spam.nu"#]; + + let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; "))); + + assert!(actual.err.contains("cyclical")); + }) +} + +#[test] +fn module_cyclical_imports_2() { + Playground::setup("module_cyclical_imports_2", |dirs, sandbox| { + sandbox + .with_files(vec![FileWithContentToBeTrimmed( + "spam.nu", + r#" + use eggs.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "eggs.nu", + r#" + use spam.nu + "#, + )]); + + let inp = &[r#"use spam.nu"#]; + + let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; "))); + + assert!(actual.err.contains("cyclical")); + }) +} + +#[test] +fn module_cyclical_imports_3() { + Playground::setup("module_cyclical_imports_3", |dirs, sandbox| { + sandbox + .with_files(vec![FileWithContentToBeTrimmed( + "spam.nu", + r#" + use eggs.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "eggs.nu", + r#" + use bacon.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "bacon.nu", + r#" + use spam.nu + "#, + )]); + + let inp = &[r#"use spam.nu"#]; + + let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; "))); + + assert!(actual.err.contains("cyclical")); + }) +}