From acc9f478f10e51e35dc27272af73df757a90fcde Mon Sep 17 00:00:00 2001 From: l3ops Date: Wed, 5 Oct 2022 15:02:51 +0200 Subject: [PATCH 1/2] fix(rome_service): disallow large files from being parsed and analyzed --- crates/rome_cli/src/traversal.rs | 14 +-- crates/rome_cli/tests/main.rs | 88 +++++++++++++++++++ .../snapshots/main_check/file_too_large.snap | 19 ++++ .../snapshots/main_ci/file_too_large.snap | 19 ++++ .../snapshots/main_format/file_too_large.snap | 19 ++++ crates/rome_console/src/fmt.rs | 64 ++++++++++++++ crates/rome_fs/src/fs/memory.rs | 5 ++ crates/rome_service/src/lib.rs | 10 +++ crates/rome_service/src/workspace/server.rs | 12 +++ 9 files changed, 240 insertions(+), 10 deletions(-) create mode 100644 crates/rome_cli/tests/snapshots/main_check/file_too_large.snap create mode 100644 crates/rome_cli/tests/snapshots/main_ci/file_too_large.snap create mode 100644 crates/rome_cli/tests/snapshots/main_format/file_too_large.snap diff --git a/crates/rome_cli/src/traversal.rs b/crates/rome_cli/src/traversal.rs index 73d5e629ca8..6e80f90d4dd 100644 --- a/crates/rome_cli/src/traversal.rs +++ b/crates/rome_cli/src/traversal.rs @@ -9,8 +9,8 @@ use rome_diagnostics::{ v2::{ self, adapters::{IoError, StdError}, - category, Advices, Category, Diagnostic, DiagnosticExt, Error, FilePath, LogCategory, - PrintDescription, PrintDiagnostic, Severity, Visit, + category, Advices, Category, Diagnostic, DiagnosticExt, Error, FilePath, PrintDescription, + PrintDiagnostic, Severity, Visit, }, MAXIMUM_DISPLAYABLE_DIAGNOSTICS, }; @@ -238,14 +238,8 @@ struct FormatDiffAdvice<'a> { impl Advices for FormatDiffAdvice<'_> { fn record(&self, visitor: &mut dyn Visit) -> io::Result<()> { - // Skip printing the diff for files over 1Mb (probably a minified file) - let max_len = self.old.len().max(self.new.len()); - if max_len >= 1_000_000 { - visitor.record_log(LogCategory::Info, &"[Diff not printed for file over 1Mb]") - } else { - let diff = TextEdit::from_unicode_words(self.old, self.new); - visitor.record_diff(&diff) - } + let diff = TextEdit::from_unicode_words(self.old, self.new); + visitor.record_diff(&diff) } } diff --git a/crates/rome_cli/tests/main.rs b/crates/rome_cli/tests/main.rs index 0d838f1c3be..0fb66a46a89 100644 --- a/crates/rome_cli/tests/main.rs +++ b/crates/rome_cli/tests/main.rs @@ -724,6 +724,34 @@ mod check { result, )); } + + #[test] + fn file_too_large() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("check.js"); + fs.insert(file_path.into(), "statement();\n".repeat(80660).as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + DynRef::Borrowed(&mut console), + Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + // Do not store the content of the file in the snapshot + fs.remove(file_path); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "file_too_large", + fs, + console, + result, + )); + } } mod ci { @@ -934,6 +962,34 @@ mod ci { result, )); } + + #[test] + fn file_too_large() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("ci.js"); + fs.insert(file_path.into(), "statement();\n".repeat(80660).as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + DynRef::Borrowed(&mut console), + Arguments::from_vec(vec![OsString::from("ci"), file_path.as_os_str().into()]), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + // Do not store the content of the file in the snapshot + fs.remove(file_path); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "file_too_large", + fs, + console, + result, + )); + } } mod format { @@ -1740,6 +1796,38 @@ mod format { result, )); } + + #[test] + fn file_too_large() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("format.js"); + fs.insert(file_path.into(), "statement();\n".repeat(80660).as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + DynRef::Borrowed(&mut console), + Arguments::from_vec(vec![ + OsString::from("format"), + file_path.as_os_str().into(), + OsString::from("--write"), + ]), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + // Do not store the content of the file in the snapshot + fs.remove(file_path); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "file_too_large", + fs, + console, + result, + )); + } } mod help { diff --git a/crates/rome_cli/tests/snapshots/main_check/file_too_large.snap b/crates/rome_cli/tests/snapshots/main_check/file_too_large.snap new file mode 100644 index 00000000000..f7dd5791f35 --- /dev/null +++ b/crates/rome_cli/tests/snapshots/main_check/file_too_large.snap @@ -0,0 +1,19 @@ +--- +source: crates/rome_cli/tests/snap_test.rs +expression: content +--- +# Emitted Messages + +```block +check.js lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × The file check.js could not be parsed because it's too large (the file is 1.0 MiB long, but the size limit is 1.0 MiB) + + +``` + +```block +Skipped 1 files +``` + + diff --git a/crates/rome_cli/tests/snapshots/main_ci/file_too_large.snap b/crates/rome_cli/tests/snapshots/main_ci/file_too_large.snap new file mode 100644 index 00000000000..c1c50158d7d --- /dev/null +++ b/crates/rome_cli/tests/snapshots/main_ci/file_too_large.snap @@ -0,0 +1,19 @@ +--- +source: crates/rome_cli/tests/snap_test.rs +expression: content +--- +# Emitted Messages + +```block +ci.js lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × The file ci.js could not be parsed because it's too large (the file is 1.0 MiB long, but the size limit is 1.0 MiB) + + +``` + +```block +Skipped 1 files +``` + + diff --git a/crates/rome_cli/tests/snapshots/main_format/file_too_large.snap b/crates/rome_cli/tests/snapshots/main_format/file_too_large.snap new file mode 100644 index 00000000000..decc1332818 --- /dev/null +++ b/crates/rome_cli/tests/snapshots/main_format/file_too_large.snap @@ -0,0 +1,19 @@ +--- +source: crates/rome_cli/tests/snap_test.rs +expression: content +--- +# Emitted Messages + +```block +format.js lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × The file format.js could not be parsed because it's too large (the file is 1.0 MiB long, but the size limit is 1.0 MiB) + + +``` + +```block +Skipped 1 files +``` + + diff --git a/crates/rome_console/src/fmt.rs b/crates/rome_console/src/fmt.rs index bb6f307fed0..b4231ac5c62 100644 --- a/crates/rome_console/src/fmt.rs +++ b/crates/rome_console/src/fmt.rs @@ -233,3 +233,67 @@ impl Display for Duration { }) } } + +#[repr(transparent)] +#[derive(Clone, Copy, Debug)] +pub struct Bytes(pub usize); + +impl std::fmt::Display for Bytes { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self(mut value) = *self; + + if value < 1024 { + return write!(fmt, "{value} B"); + } + + const PREFIX: [char; 4] = ['K', 'M', 'G', 'T']; + let prefix = PREFIX + .into_iter() + .find(|_| { + let next_value = value / 1024; + if next_value < 1024 { + return true; + } + + value = next_value; + false + }) + .unwrap_or('T'); + + write!(fmt, "{:.1} {prefix}iB", value as f32 / 1024.0) + } +} + +impl Display for Bytes { + fn fmt(&self, fmt: &mut Formatter) -> io::Result<()> { + write!(fmt, "{self}") + } +} + +#[cfg(test)] +mod tests { + use crate::fmt::Bytes; + + #[test] + fn display_bytes() { + // Examples taken from https://stackoverflow.com/a/3758880 + assert_eq!(Bytes(0).to_string(), "0 B"); + assert_eq!(Bytes(27).to_string(), "27 B"); + assert_eq!(Bytes(999).to_string(), "999 B"); + assert_eq!(Bytes(1_000).to_string(), "1000 B"); + assert_eq!(Bytes(1_023).to_string(), "1023 B"); + assert_eq!(Bytes(1_024).to_string(), "1.0 KiB"); + assert_eq!(Bytes(1_728).to_string(), "1.7 KiB"); + assert_eq!(Bytes(110_592).to_string(), "108.0 KiB"); + assert_eq!(Bytes(999_999).to_string(), "976.6 KiB"); + assert_eq!(Bytes(7_077_888).to_string(), "6.8 MiB"); + assert_eq!(Bytes(452_984_832).to_string(), "432.0 MiB"); + assert_eq!(Bytes(28_991_029_248).to_string(), "27.0 GiB"); + assert_eq!(Bytes(1_855_425_871_872).to_string(), "1.7 TiB"); + + #[cfg(target_pointer_width = "32")] + assert_eq!(Bytes(usize::MAX).to_string(), "4.0 GiB"); + #[cfg(target_pointer_width = "64")] + assert_eq!(Bytes(usize::MAX).to_string(), "16384.0 TiB"); + } +} diff --git a/crates/rome_fs/src/fs/memory.rs b/crates/rome_fs/src/fs/memory.rs index 40c686c4e5f..b15c3272573 100644 --- a/crates/rome_fs/src/fs/memory.rs +++ b/crates/rome_fs/src/fs/memory.rs @@ -60,6 +60,11 @@ impl MemoryFileSystem { self.errors.insert(path, kind); } + /// Remove a file from the filesystem + pub fn remove(&mut self, path: &Path) { + self.files.0.write().remove(path); + } + pub fn files(self) -> IntoIter { let files = self.files.0.into_inner(); files.into_iter() diff --git a/crates/rome_service/src/lib.rs b/crates/rome_service/src/lib.rs index 397fccf0a17..0b5d7e9a131 100644 --- a/crates/rome_service/src/lib.rs +++ b/crates/rome_service/src/lib.rs @@ -1,3 +1,4 @@ +use rome_console::fmt::Bytes; use rome_console::{Console, EnvConsole}; use rome_formatter::FormatError; use rome_fs::{FileSystem, OsFileSystem, RomePath}; @@ -71,6 +72,12 @@ pub enum RomeError { TransportError(TransportError), /// Emitted when the file is ignored and should not be processed FileIgnored(PathBuf), + /// Emitted when a file could not be parsed because it's larger than the size limite + FileTooLarge { + path: PathBuf, + size: usize, + limit: usize, + }, } impl Debug for RomeError { @@ -163,6 +170,9 @@ impl Display for RomeError { RomeError::FileIgnored(path) => { write!(f, "The file {} was ignored", path.display()) } + RomeError::FileTooLarge { path, size, limit } => { + write!(f, "The file {} could not be parsed because it's too large (the file is {} long, but the size limit is {})", path.display(), Bytes(*size), Bytes(*limit)) + } } } } diff --git a/crates/rome_service/src/workspace/server.rs b/crates/rome_service/src/workspace/server.rs index d09682945f6..30707b19806 100644 --- a/crates/rome_service/src/workspace/server.rs +++ b/crates/rome_service/src/workspace/server.rs @@ -169,6 +169,18 @@ impl WorkspaceServer { .parse .ok_or_else(self.build_capability_error(rome_path))?; + /// Limit the size of files to 1.0 MiB + const SIZE_LIMIT: usize = 1024 * 1024; + + let size = document.content.as_bytes().len(); + if size >= SIZE_LIMIT { + return Err(RomeError::FileTooLarge { + path: rome_path.to_path_buf(), + size, + limit: SIZE_LIMIT, + }); + } + let parsed = parse(rome_path, document.language_hint, &document.content); Ok(entry.insert(parsed).clone()) From 47e475e21e036101beaa35a5147015263313a32f Mon Sep 17 00:00:00 2001 From: l3ops Date: Wed, 5 Oct 2022 16:09:10 +0200 Subject: [PATCH 2/2] address PR review --- crates/rome_service/src/workspace/server.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/rome_service/src/workspace/server.rs b/crates/rome_service/src/workspace/server.rs index 30707b19806..bf2d90058ee 100644 --- a/crates/rome_service/src/workspace/server.rs +++ b/crates/rome_service/src/workspace/server.rs @@ -170,14 +170,14 @@ impl WorkspaceServer { .ok_or_else(self.build_capability_error(rome_path))?; /// Limit the size of files to 1.0 MiB - const SIZE_LIMIT: usize = 1024 * 1024; + const SIZE_LIMIT_IN_BYTES: usize = 1024 * 1024; let size = document.content.as_bytes().len(); - if size >= SIZE_LIMIT { + if size >= SIZE_LIMIT_IN_BYTES { return Err(RomeError::FileTooLarge { path: rome_path.to_path_buf(), size, - limit: SIZE_LIMIT, + limit: SIZE_LIMIT_IN_BYTES, }); }