diff --git a/.devcontainer/postCreateCommand.sh b/.devcontainer/postCreateCommand.sh index 7a3d044f..51c6d633 100755 --- a/.devcontainer/postCreateCommand.sh +++ b/.devcontainer/postCreateCommand.sh @@ -1,9 +1,4 @@ #!/usr/bin/env bash -echo "Build NPM packages" -cd client -npm update -npm run build -echo "Build server." -cd ../server -cargo build +cd server +../bt install --dev \ No newline at end of file diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 26739c76..c9e96cae 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -63,7 +63,7 @@ jobs: # we specify bash to get pipefail; it guards against the `curl` command # failing. otherwise `sh` won't catch that `curl` returned non-0 shell: bash - run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.27.1/cargo-dist-installer.sh | sh" + run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.28.0/cargo-dist-installer.sh | sh" - name: Cache dist uses: actions/upload-artifact@v4 with: diff --git a/builder/src/main.rs b/builder/src/main.rs index f8ca839a..de062a0f 100644 --- a/builder/src/main.rs +++ b/builder/src/main.rs @@ -29,7 +29,7 @@ // ## Imports // // ### Standard library -use std::{ffi::OsStr, fs, path::Path, process::Command}; +use std::{ffi::OsStr, fs, io, path::Path, process::Command}; // ### Third-party use clap::{Parser, Subcommand}; @@ -104,7 +104,7 @@ fn run_script, P: AsRef + std::fmt::Display>( // True to report errors based on the process' exit code; false to ignore // the code. check_exit_code: bool, -) -> Result<(), Box> { +) -> io::Result<()> { let mut process; if cfg!(windows) { process = Command::new("cmd"); @@ -120,7 +120,10 @@ fn run_script, P: AsRef + std::fmt::Display>( if exit_code == Some(0) || (exit_code.is_some() && !check_exit_code) { Ok(()) } else { - Err("npm exit code indicates failure".into()) + Err(io::Error::new( + io::ErrorKind::Other, + "npm exit code indicates failure.", + )) } } @@ -129,11 +132,7 @@ fn run_script, P: AsRef + std::fmt::Display>( /// programs (`robocopy`/`rsync`) to accomplish this. Very important: the `src` /// **must** end with a `/`, otherwise the Windows and Linux copies aren't /// identical. -fn quick_copy_dir>( - src: P, - dest: P, - files: Option

, -) -> Result<(), Box> { +fn quick_copy_dir>(src: P, dest: P, files: Option

) -> io::Result<()> { assert!(src.as_ref().to_string_lossy().ends_with('/')); let mut copy_process; #[cfg(windows)] @@ -206,29 +205,26 @@ fn quick_copy_dir>( println!("{:#?}", ©_process); // Check for errors. - let exit_status = copy_process - .status() - .map_err(|err| -> String { format!("Error running copy process: {err}") })?; - let exit_code = exit_status + let exit_code = copy_process + .status()? .code() .expect("Copy process terminated by signal"); // Per // [these docs](https://learn.microsoft.com/en-us/troubleshoot/windows-server/backup-and-storage/return-codes-used-robocopy-utility), // check the return code. if cfg!(windows) && exit_code >= 8 || !cfg!(windows) && exit_code != 0 { - Err(format!("Copy process return code {exit_code} indicates failure.").into()) + Err(io::Error::new( + io::ErrorKind::Other, + format!("Copy process return code {exit_code} indicates failure."), + )) } else { Ok(()) } } -fn remove_dir_all_if_exists + std::fmt::Display>( - path: P, -) -> Result<(), Box> { +fn remove_dir_all_if_exists + std::fmt::Display>(path: P) -> io::Result<()> { if Path::new(path.as_ref()).try_exists().unwrap() { - if let Err(err) = fs::remove_dir_all(path.as_ref()) { - return Err(format!("Error removing directory tree {path}: {err}").into()); - } + fs::remove_dir_all(path.as_ref())?; } Ok(()) @@ -242,19 +238,20 @@ fn search_and_replace_file< path: P, search_regex: S1, replace_string: S2, -) -> Result<(), Box> { - let file_contents = fs::read_to_string(&path) - .map_err(|err| -> String { format!("Unable to open file {path} for reading: {err}") })?; - let re = Regex::new(search_regex.as_ref()) - .map_err(|err| -> String { format!("Error in search regex {search_regex}: {err}") })?; +) -> io::Result<()> { + let file_contents = fs::read_to_string(&path)?; + let re = Regex::new(search_regex.as_ref()).map_err(|err| { + io::Error::new( + io::ErrorKind::Other, + format!("Error in search regex {search_regex}: {err}"), + ) + })?; let file_contents_replaced = re.replace(&file_contents, replace_string.as_ref()); assert_ne!( file_contents, file_contents_replaced, "No replacements made." ); fs::write(&path, file_contents_replaced.as_bytes()) - .map_err(|err| -> String { format!("Error writing to {path}: {err}") })?; - Ok(()) } // ## Core routines @@ -262,7 +259,7 @@ fn search_and_replace_file< // These functions simplify common build-focused development tasks and support // CI builds. /// After updating files in the client's Node files, perform some fix-ups. -fn patch_client_npm() -> Result<(), Box> { +fn patch_client_npm() -> io::Result<()> { // Apply a the fixes described in // [issue 27](https://github.com/bjones1/CodeChat_Editor/issues/27). // @@ -310,7 +307,7 @@ fn patch_client_npm() -> Result<(), Box> { Ok(()) } -fn run_install(dev: bool) -> Result<(), Box> { +fn run_install(dev: bool) -> io::Result<()> { run_script("npm", &["install"], "../client", true)?; patch_client_npm()?; run_script("npm", &["install"], "../extensions/VSCode", true)?; @@ -331,7 +328,7 @@ fn run_install(dev: bool) -> Result<(), Box> { Ok(()) } -fn run_update() -> Result<(), Box> { +fn run_update() -> io::Result<()> { run_script("npm", &["update"], "../client", true)?; patch_client_npm()?; run_script("npm", &["update"], "../extensions/VSCode", true)?; @@ -349,7 +346,7 @@ fn run_update() -> Result<(), Box> { Ok(()) } -fn run_test() -> Result<(), Box> { +fn run_test() -> io::Result<()> { // On Windows, `cargo sort --check` fails since it default to LF, not CRLF, // line endings. Work around this by changing this setting only on Windows. // See the @@ -392,7 +389,7 @@ fn run_test() -> Result<(), Box> { Ok(()) } -fn run_build() -> Result<(), Box> { +fn run_build() -> io::Result<()> { // Clean out all bundled files before the rebuild. remove_dir_all_if_exists("../client/static/bundled")?; run_script("npm", &["run", "build"], "../client", true)?; @@ -404,7 +401,7 @@ fn run_build() -> Result<(), Box> { Ok(()) } -fn run_change_version(new_version: &String) -> Result<(), Box> { +fn run_change_version(new_version: &String) -> io::Result<()> { let replacement_string = format!("${{1}}{new_version}${{2}}"); search_and_replace_file( "Cargo.toml", @@ -425,7 +422,7 @@ fn run_change_version(new_version: &String) -> Result<(), Box Result<(), Box> { +fn run_prerelease() -> io::Result<()> { // Clean out all bundled files before the rebuild. remove_dir_all_if_exists("../client/static/bundled")?; run_install(true)?; @@ -433,7 +430,7 @@ fn run_prerelease() -> Result<(), Box> { Ok(()) } -fn run_postrelease(target: &str) -> Result<(), Box> { +fn run_postrelease(target: &str) -> io::Result<()> { let server_dir = "../extensions/VSCode/server"; // Only clean the `server/` directory if it exists. remove_dir_all_if_exists(server_dir)?; @@ -468,7 +465,7 @@ fn run_postrelease(target: &str) -> Result<(), Box> { // The following code implements the command-line interface for the CodeChat // Editor. impl Cli { - fn run(self) -> Result<(), Box> { + fn run(self) -> io::Result<()> { match &self.command { Commands::Install { dev } => run_install(*dev), Commands::Update => run_update(), @@ -481,7 +478,7 @@ impl Cli { } } -fn main() -> Result<(), Box> { +fn main() -> io::Result<()> { let cli = Cli::parse(); cli.run()?; diff --git a/docs/changelog.md b/docs/changelog.md index 8fee3309..54fdc6e9 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -19,7 +19,8 @@ CodeChat Editor. If not, see # Changelog - [Github master](https://github.com/bjones1/CodeChat_Editor): - - No changes. + - Correctly handle file not found in VSCode. + - Correct filename handling on Windows. - v0.1.7, 2025-Jan-08: - Fixed hyperlink navigation. - Fixed case-insensitive filename handling bugs. diff --git a/extensions/VSCode/src/extension.ts b/extensions/VSCode/src/extension.ts index 3913d19b..5197b7cf 100644 --- a/extensions/VSCode/src/extension.ts +++ b/extensions/VSCode/src/extension.ts @@ -16,9 +16,8 @@ // // # `extension.ts` - The CodeChat Editor Visual Studio Code extension // -// This extension creates a webview (see `activation/deactivation`\_), then uses -// a websocket connection to the CodeChat Editor Server and Client to render -// editor text in that webview. +// This extension creates a webview, then uses a websocket connection to the +// CodeChat Editor Server and Client to render editor text in that webview. // // ## Imports // @@ -237,7 +236,8 @@ export const activate = (context: vscode.ExtensionContext) => { // column. viewColumn: vscode.ViewColumn.Beside, }, - // See WebViewOptions\_. + // See + // [WebViewOptions](https://code.visualstudio.com/api/references/vscode-api#WebviewOptions). { enableScripts: true, // Per the @@ -257,9 +257,6 @@ export const activate = (context: vscode.ExtensionContext) => { retainContextWhenHidden: true, } ); - // TODO: do I need to dispose of this and the following - // event handlers? I'm assuming that it will be done - // automatically when the object is disposed. webview_panel.onDidDispose(async () => { // Shut down the render client when the webview // panel closes. @@ -436,14 +433,20 @@ export const activate = (context: vscode.ExtensionContext) => { const current_file = value as string; vscode.workspace .openTextDocument(current_file) - .then((document) => { - ignore_active_editor_change = true; - vscode.window.showTextDocument( - document, - current_editor?.viewColumn - ); - send_result(id); - }); + .then( + (document) => { + ignore_active_editor_change = true; + vscode.window.showTextDocument( + document, + current_editor?.viewColumn + ); + send_result(id); + }, + (reason) => + send_result(id, { + Err: `Error: unable to open file ${current_file}: ${reason}`, + }) + ); break; } diff --git a/server/Cargo.toml b/server/Cargo.toml index 9657819c..d298124e 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -54,7 +54,7 @@ mime = "0.3.17" mime_guess = "2.0.5" minreq = "2.12.0" normalize-line-endings = "0.3.0" -notify-debouncer-full = "0.4" +notify-debouncer-full = "0.5" open = "5.3.0" path-slash = "0.2.1" pest = "2.7.14" diff --git a/server/dist-workspace.toml b/server/dist-workspace.toml index b3a69016..f90aeff1 100644 --- a/server/dist-workspace.toml +++ b/server/dist-workspace.toml @@ -23,7 +23,7 @@ members = ["cargo:."] # Config for 'dist' [dist] # The preferred dist version to use in CI (Cargo.toml SemVer syntax) -cargo-dist-version = "0.27.1" +cargo-dist-version = "0.28.0" # Extra static files to include in each App (path relative to this Cargo.toml's dir) include = ["log4rs.yml", "hashLocations.json", "../client/static"] # The installers to generate for each app diff --git a/server/src/processing.rs b/server/src/processing.rs index ac8c800d..661936e0 100644 --- a/server/src/processing.rs +++ b/server/src/processing.rs @@ -150,11 +150,12 @@ pub fn find_path_to_toc(file_path: &Path) -> Option { // named `toc.md`. let mut path_to_toc = PathBuf::new(); let mut current_dir = file_path.to_path_buf(); + // Drop the last element (the current file name) from the search. + current_dir.pop(); loop { let mut project_file = current_dir.clone(); project_file.push("toc.md"); if project_file.is_file() { - path_to_toc.pop(); path_to_toc.push("toc.md"); return Some(path_to_toc); } diff --git a/server/src/webserver.rs b/server/src/webserver.rs index 56023321..89affcb8 100644 --- a/server/src/webserver.rs +++ b/server/src/webserver.rs @@ -486,11 +486,16 @@ pub async fn filesystem_endpoint( req: &HttpRequest, app_state: &web::Data, ) -> HttpResponse { - let (connection_id, file_path) = request_path.into_inner(); - let file_path = match try_canonicalize(&file_path) { + let (connection_id, request_file_path) = request_path.into_inner(); + // On Windows, backslashes in the `request_file_path` will be treated as + // path separators; however, HTTP does not treat them as path separators. + // Therefore, re-encode them to prevent inconsistency between the way HTTP + // and this program interpret file paths. + let fixed_file_path = request_file_path.replace("\\", "%5C"); + let file_path = match try_canonicalize(&fixed_file_path) { Ok(v) => v, Err(err) => { - let msg = format!("Error: unable to convert path {file_path}: {err}."); + let msg = format!("Error: unable to convert path {request_file_path}: {err}."); error!("{msg}"); return html_not_found(&msg); } @@ -1174,51 +1179,69 @@ async fn send_response(client_tx: &Sender, id: f64, result: Messa } } -fn url_to_path(url_string: &str, expected_prefix: &[&str]) -> Result { - // Convert this URL back to a file path. - match Url::parse(url_string) { - Err(err) => Err(format!("Error: unable to parse URL {url_string}: {err}")), - Ok(url) => match url.path_segments() { - None => Err(format!("Error: URL {url} cannot be a base.")), - Some(path_segments) => { - // Make sure the path segments start with the `expected_prefix`. - let path_segments_vec: Vec<_> = path_segments.collect(); - let prefix_equal = expected_prefix - .iter() - .zip(&path_segments_vec) - .all(|(a, b)| a == b); - // The URL should have at least the expected prefix plus one - // more element (the connection ID). - if path_segments_vec.len() < expected_prefix.len() + 1 || !prefix_equal { - Err(format!("Error: URL {url} has incorrect prefix.")) - } else { - // Strip these first three segments; the remainder is a file - // path. - let path_str_encoded = - path_segments_vec[expected_prefix.len() + 1..].join(MAIN_SEPARATOR_STR); - match urlencoding::decode(&path_str_encoded) { - Err(err) => { - Err(format!("Error: unable to decode URL {url_string}: {err}.")) - } - Ok(path_str) => { - // On non-Windows systems, the path should start - // with a `/`. Windows paths should already start - // with a drive letter. - #[cfg(not(target_os = "windows"))] - let path_str = "/".to_string() + &path_str; - try_canonicalize(&path_str) - } - } - } - } - }, +// Convert a URL referring to a file in the filesystem into the path to that +// file. +fn url_to_path( + // The URL for the file. + url_string: &str, + // An array of URL path segments; the URL must start with these. They will + // be dropped from the resulting file's path. + expected_prefix: &[&str], + // Output: the resulting path to the file, or a string explaining why an + // error occurred during conversion. +) -> Result { + // Parse to a URL, then split it to path segments. + let url = Url::parse(url_string) + .map_err(|e| format!("Error: unable to parse URL {url_string}: {e}"))?; + let path_segments_vec: Vec<_> = url + .path_segments() + .ok_or_else(|| format!("Error: URL {url} cannot be a base."))? + .collect(); + + // Make sure the path segments start with the `expected_prefix`. + let prefix_equal = expected_prefix + .iter() + .zip(&path_segments_vec) + .all(|(a, b)| a == b); + // The URL should have at least the expected prefix plus one more element + // (the connection ID). + if path_segments_vec.len() < expected_prefix.len() + 1 || !prefix_equal { + return Err(format!("Error: URL {url} has incorrect prefix.")); } + + // Strip the expected prefix; the remainder is a file path. + let path_segments_suffix = path_segments_vec[expected_prefix.len() + 1..].to_vec(); + + // URL decode each segment; however, re-encode the `\`, since this isn't a + // valid path separator in a URL but is incorrectly treated as such on + // Windows. + let path_segments_suffix_decoded = path_segments_suffix + .iter() + .map(|path_segment| { + urlencoding::decode(path_segment) + .map_err(|e| format!("Error: unable to decode URL {url_string}: {e}.")) + .map(|path_seg| path_seg.replace("\\", "%5C")) + }) + .collect::, String>>()?; + + // Join the segments into a path. + let path_str = path_segments_suffix_decoded.join(MAIN_SEPARATOR_STR); + + // On non-Windows systems, the path should start with a `/`. Windows paths + // should already start with a drive letter. + #[cfg(not(target_os = "windows"))] + let path_str = "/".to_string() + &path_str; + + try_canonicalize(&path_str) } -// Given a string representing a file, transform it into a `PathBuf`. Correct it as much as possible: +// Given a string representing a file, transform it into a `PathBuf`. Correct it +// as much as possible: // -// 1. Convert Linux path separators to this platform's path separators. -// 2. If the file exists and if this is Windows, correct case based on the actual file's naming (even though the filesystem is case-insensitive; this makes comparisons in the TypeScript simpler). +// 1. Convert Linux path separators to this platform's path separators. +// 2. If the file exists and if this is Windows, correct case based on the +// actual file's naming (even though the filesystem is case-insensitive; +// this makes comparisons in the TypeScript simpler). fn try_canonicalize(file_path: &str) -> Result { match PathBuf::from_str(file_path) { Err(err) => Err(format!( @@ -1226,12 +1249,10 @@ fn try_canonicalize(file_path: &str) -> Result { )), Ok(path_buf) => match path_buf.canonicalize() { // [Canonicalize](https://doc.rust-lang.org/stable/std/fs/fn.canonicalize.html#errors) - // fails if the path doesn't exist. For - // unsaved files, this is expected; - // therefore, use + // fails if the path doesn't exist. For unsaved files, this is + // expected; therefore, use // [absolute](https://doc.rust-lang.org/stable/std/path/fn.absolute.html) - // on error, since it doesn't require the - // path to exist. + // on error, since it doesn't require the path to exist. Err(_) => match path::absolute(&path_buf) { Err(err) => Err(format!("Unable to make {path_buf:?} absolute: {err}")), Ok(p) => Ok(p), diff --git a/server/src/webserver/filewatcher.rs b/server/src/webserver/filewatcher.rs index ca16ba89..8a30ae44 100644 --- a/server/src/webserver/filewatcher.rs +++ b/server/src/webserver/filewatcher.rs @@ -816,7 +816,7 @@ mod tests { send_response(&ide_tx_queue, id, Ok(ResultOkTypes::Void)).await; // 2. After fetching the file, we should get an update. - let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_string_lossy()); + let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_slash().unwrap()); let req = test::TestRequest::get().uri(&uri).to_request(); let resp = test::call_service(&app, req).await; assert!(resp.status().is_success()); @@ -855,7 +855,7 @@ mod tests { .to_str() .unwrap() .to_string(); - let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_string_lossy()); + let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_slash().unwrap()); let req = test::TestRequest::get().uri(&uri).to_request(); let resp = test::call_service(&app, req).await; assert!(resp.status().is_success()); diff --git a/server/src/webserver/tests.rs b/server/src/webserver/tests.rs index b86366e9..7c13a259 100644 --- a/server/src/webserver/tests.rs +++ b/server/src/webserver/tests.rs @@ -56,6 +56,21 @@ fn test_url_to_path() { ),)) ); + // Test a path with a backslash in it. + assert_eq!( + url_to_path( + &format!( + "http://127.0.0.1:8080/fw/fsc/dummy_connection_id/{}foo%5Cbar.py", + if cfg!(windows) { "C:/" } else { "" } + ), + FILEWATCHER_PATH_PREFIX + ), + Ok(PathBuf::from(format!( + "{}foo%5Cbar.py", + if cfg!(windows) { "C:\\" } else { "/" } + ),)) + ); + // Test an actual path. let test_dir_str = test_dir.to_str().unwrap(); assert_eq!( diff --git a/server/src/webserver/vscode.rs b/server/src/webserver/vscode.rs index 687427cf..5493a6d2 100644 --- a/server/src/webserver/vscode.rs +++ b/server/src/webserver/vscode.rs @@ -398,7 +398,9 @@ pub async fn vscode_ide_websocket( } } }; - // If there's an error, then report it; otherwise, the message is passed to the Client, which will provide the result. + // If there's an error, then report it; + // otherwise, the message is passed to the + // Client, which will provide the result. if let Err(err) = &result { error!("{err}"); send_response(&to_ide_tx, ide_message.id, result).await; @@ -977,6 +979,62 @@ mod test { temp_dir.close().unwrap(); } + // Fetch a file that exists, but using backslashes. This should still fail, + // even on Windows. + #[actix_web::test] + async fn test_vscode_ide_websocket3a() { + let connection_id = "test-connection-id3a"; + let (temp_dir, test_dir, mut ws_ide, _) = prep_test!(connection_id).await; + open_client(&mut ws_ide).await; + + let file_path = test_dir.join("test.py"); + // Force the path separator to be Window-style for this test, even on + // non-Windows platforms. + let file_path_str = file_path.to_str().unwrap().to_string().replace("/", "\\"); + + // Do this is a thread, since the request generates a message that + // requires a response in order to complete. + let file_path_str_thread = file_path_str.clone(); + let join_handle = thread::spawn(move || { + assert_eq!( + minreq::get(format!( + "http://localhost:8080/vsc/fs/{connection_id}/{}", + file_path_str_thread + )) + .send() + .unwrap() + .status_code, + 404 + ) + }); + + // The HTTP request produces a `LoadFile` message. + // + // Message ids: IDE - 4, Server - 3->6, Client - 2. + let em = read_message(&mut ws_ide).await; + cast!(em.message, EditorMessageContents::LoadFile); + // Skip comparing the file names, due to the backslash encoding. + assert_eq!(em.id, 3.0); + + // Reply to the `LoadFile` message -- the file isn't present. + send_message( + &mut ws_ide, + &EditorMessage { + id: 3.0, + message: EditorMessageContents::Result(Ok(ResultOkTypes::LoadFile(None))), + }, + ) + .await; + + // This should cause the HTTP request to complete by receiving the + // response (file not found). + join_handle.join().unwrap(); + + check_logger_errors(0); + // Report any errors produced when removing the temporary directory. + temp_dir.close().unwrap(); + } + // Send a `CurrentFile` message with a file to edit that exists only in the // IDE. #[actix_web::test] @@ -1360,7 +1418,8 @@ mod test { minreq::get(format!( "http://localhost:8080/vsc/fs/{connection_id}/{}/{}", test_dir_thread.to_slash().unwrap(), - // On Windows, send incorrect case for this file; the server should correct it. + // On Windows, send incorrect case for this file; the server + // should correct it. if cfg!(windows) { "Test.py" } else { "test.py" } )) .send() diff --git a/server/tests/fixtures/webserver/vscode/test/test_vscode_ide_websocket3a/test.py b/server/tests/fixtures/webserver/vscode/test/test_vscode_ide_websocket3a/test.py new file mode 100644 index 00000000..36a317ed --- /dev/null +++ b/server/tests/fixtures/webserver/vscode/test/test_vscode_ide_websocket3a/test.py @@ -0,0 +1 @@ +# test.py \ No newline at end of file