From f552917b0ebd71201de2b0dd521331b0ce4343ed Mon Sep 17 00:00:00 2001 From: Angie Date: Mon, 12 Aug 2024 15:44:20 -0400 Subject: [PATCH] Make sure paths are always escaped --- slinky-cli/src/main.rs | 11 ++-- slinky/src/escaped_path.rs | 51 +++++++++++++++ slinky/src/file_info.rs | 15 ++++- slinky/src/lib.rs | 2 + slinky/src/linker_writer.rs | 98 ++++++++++++----------------- slinky/src/partial_linker_writer.rs | 44 ++++--------- slinky/src/runtime_settings.rs | 7 ++- slinky/src/segment.rs | 6 +- slinky/src/settings.rs | 50 ++++++++++++++- slinky/src/traits.rs | 6 +- slinky/tests/integration_test.rs | 31 +++++---- 11 files changed, 208 insertions(+), 113 deletions(-) create mode 100644 slinky/src/escaped_path.rs diff --git a/slinky-cli/src/main.rs b/slinky-cli/src/main.rs index 289e33c..3a51d85 100644 --- a/slinky-cli/src/main.rs +++ b/slinky-cli/src/main.rs @@ -5,7 +5,7 @@ use std::{error::Error, path::PathBuf}; use clap::Parser; use regex::Regex; -use slinky::ScriptGenerator; +use slinky::{RuntimeSettings, ScriptGenerator}; // TODO: Add program description to cli @@ -69,13 +69,16 @@ fn create_runtime_settings(cli: &Cli) -> slinky::RuntimeSettings { fn write_script( writer: &mut impl ScriptGenerator, document: &slinky::Document, + rs: &RuntimeSettings, output: &Option, ) { writer.add_whole_document(document).expect("ah?"); if let Some(output_path) = output { writer - .export_linker_script_to_file(output_path) + .export_linker_script_to_file( + &rs.escape_path(output_path).expect("Error escaping path"), + ) .expect("Error writing the linker script"); } else { println!( @@ -104,10 +107,10 @@ fn main() { if cli.partial_linking { let mut writer = slinky::PartialLinkerWriter::new(&document, &rs); - write_script(&mut writer, &document, &cli.output); + write_script(&mut writer, &document, &rs, &cli.output); } else { let mut writer = slinky::LinkerWriter::new(&document, &rs); - write_script(&mut writer, &document, &cli.output); + write_script(&mut writer, &document, &rs, &cli.output); } } diff --git a/slinky/src/escaped_path.rs b/slinky/src/escaped_path.rs new file mode 100644 index 0000000..3aa72b3 --- /dev/null +++ b/slinky/src/escaped_path.rs @@ -0,0 +1,51 @@ +/* SPDX-FileCopyrightText: © 2024 decompals */ +/* SPDX-License-Identifier: MIT */ + +use std::path::{Path, PathBuf}; + +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct EscapedPath(pub PathBuf); + +impl AsRef for EscapedPath { + fn as_ref(&self) -> &PathBuf { + &self.0 + } +} + +impl AsRef for EscapedPath { + fn as_ref(&self) -> &Path { + &self.0 + } +} + +impl AsMut for EscapedPath { + fn as_mut(&mut self) -> &mut PathBuf { + &mut self.0 + } +} + +impl From for EscapedPath { + fn from(value: String) -> Self { + Self(PathBuf::from(value)) + } +} + +impl Default for EscapedPath { + fn default() -> Self { + Self::new() + } +} + +impl EscapedPath { + pub fn new() -> Self { + Self(PathBuf::new()) + } + + pub fn push(&mut self, path: EscapedPath) { + self.0.push(path.0) + } + + pub fn display(&self) -> std::path::Display { + self.0.display() + } +} diff --git a/slinky/src/file_info.rs b/slinky/src/file_info.rs index 0ab10ca..4798b32 100644 --- a/slinky/src/file_info.rs +++ b/slinky/src/file_info.rs @@ -7,7 +7,10 @@ use std::{ path::{Path, PathBuf}, }; -use crate::{absent_nullable::AbsentNullable, file_kind::FileKind, Settings, SlinkyError}; +use crate::{ + absent_nullable::AbsentNullable, file_kind::FileKind, EscapedPath, RuntimeSettings, Settings, + SlinkyError, +}; #[derive(PartialEq, Debug, Clone)] pub struct FileInfo { @@ -55,6 +58,16 @@ impl FileInfo { } } +impl FileInfo { + pub fn path_escaped(&self, rs: &RuntimeSettings) -> Result { + rs.escape_path(&self.path) + } + + pub fn dir_escaped(&self, rs: &RuntimeSettings) -> Result { + rs.escape_path(&self.dir) + } +} + #[derive(Deserialize, PartialEq, Debug)] #[serde(deny_unknown_fields)] pub(crate) struct FileInfoSerial { diff --git a/slinky/src/lib.rs b/slinky/src/lib.rs index ef72c6b..722ee67 100644 --- a/slinky/src/lib.rs +++ b/slinky/src/lib.rs @@ -3,6 +3,7 @@ mod absent_nullable; mod error; +mod escaped_path; mod traits; mod utils; @@ -28,6 +29,7 @@ mod runtime_settings; pub mod version; pub use error::SlinkyError; +pub use escaped_path::EscapedPath; pub use linker_symbols_style::LinkerSymbolsStyle; pub use settings::Settings; diff --git a/slinky/src/linker_writer.rs b/slinky/src/linker_writer.rs index f114fff..9f5cd8f 100644 --- a/slinky/src/linker_writer.rs +++ b/slinky/src/linker_writer.rs @@ -1,12 +1,11 @@ /* SPDX-FileCopyrightText: © 2024 decompals */ /* SPDX-License-Identifier: MIT */ -use std::path::PathBuf; -use std::{io::Write, path::Path}; +use std::io::Write; use crate::{ - utils, version, Document, FileInfo, FileKind, RuntimeSettings, ScriptExporter, ScriptGenerator, - ScriptImporter, Segment, SlinkyError, SymbolAssignment, VramClass, + utils, version, Document, EscapedPath, FileInfo, FileKind, RuntimeSettings, ScriptExporter, + ScriptGenerator, ScriptImporter, Segment, SlinkyError, SymbolAssignment, VramClass, }; use crate::script_buffer::ScriptBuffer; @@ -15,7 +14,7 @@ pub struct LinkerWriter<'a> { buffer: ScriptBuffer, // Used for dependency generation - files_paths: indexmap::IndexSet, + files_paths: indexmap::IndexSet, vram_classes: indexmap::IndexMap, @@ -115,8 +114,8 @@ impl ScriptImporter for LinkerWriter<'_> { } impl ScriptExporter for LinkerWriter<'_> { - fn export_linker_script_to_file(&self, path: &Path) -> Result<(), SlinkyError> { - let mut f = utils::create_file_and_parents(path)?; + fn export_linker_script_to_file(&self, path: &EscapedPath) -> Result<(), SlinkyError> { + let mut f = utils::create_file_and_parents(path.as_ref())?; self.export_linker_script(&mut f) } @@ -135,17 +134,14 @@ impl ScriptExporter for LinkerWriter<'_> { } fn save_other_files(&self) -> Result<(), SlinkyError> { - if let Some(d_path) = &self.d.settings.d_path { - if let Some(target_path) = &self.d.settings.target_path { - self.export_dependencies_file_to_file( - &self.rs.escape_path(d_path)?, - &self.rs.escape_path(target_path)?, - )?; + if let Some(d_path) = &self.d.settings.d_path_escaped(self.rs)? { + if let Some(target_path) = &self.d.settings.target_path_escaped(self.rs)? { + self.export_dependencies_file_to_file(d_path, target_path)?; } } - if let Some(symbols_header_path) = &self.d.settings.symbols_header_path { - self.export_symbol_header_to_file(&self.rs.escape_path(symbols_header_path)?)?; + if let Some(symbols_header_path) = &self.d.settings.symbols_header_path_escaped(self.rs)? { + self.export_symbol_header_to_file(symbols_header_path)?; } Ok(()) @@ -173,7 +169,7 @@ impl LinkerWriter<'_> { pub fn export_dependencies_file( &self, dst: &mut impl Write, - target_path: &Path, + target_path: &EscapedPath, ) -> Result<(), SlinkyError> { if self.rs.emit_version_comment() { if let Err(e) = write!( @@ -190,20 +186,18 @@ impl LinkerWriter<'_> { } } - let escaped_target_path = self.rs.escape_path(target_path)?; - if let Err(e) = write!(dst, "{}:", escaped_target_path.display()) { + if let Err(e) = write!(dst, "{}:", target_path.display()) { return Err(SlinkyError::FailedWrite { description: e.to_string(), - contents: escaped_target_path.display().to_string(), + contents: target_path.display().to_string(), }); } for p in &self.files_paths { - let escaped_p = self.rs.escape_path(p)?; - if let Err(e) = write!(dst, " \\\n {}", escaped_p.display()) { + if let Err(e) = write!(dst, " \\\n {}", p.display()) { return Err(SlinkyError::FailedWrite { description: e.to_string(), - contents: escaped_p.display().to_string(), + contents: p.display().to_string(), }); } } @@ -216,11 +210,10 @@ impl LinkerWriter<'_> { } for p in &self.files_paths { - let escaped_p = self.rs.escape_path(p)?; - if let Err(e) = writeln!(dst, "{}:", escaped_p.display()) { + if let Err(e) = writeln!(dst, "{}:", p.display()) { return Err(SlinkyError::FailedWrite { description: e.to_string(), - contents: escaped_p.display().to_string(), + contents: p.display().to_string(), }); } } @@ -230,17 +223,17 @@ impl LinkerWriter<'_> { pub fn export_dependencies_file_to_file( &self, - path: &Path, - target_path: &Path, + path: &EscapedPath, + target_path: &EscapedPath, ) -> Result<(), SlinkyError> { - let mut f = utils::create_file_and_parents(path)?; + let mut f = utils::create_file_and_parents(path.as_ref())?; self.export_dependencies_file(&mut f, target_path) } pub fn export_dependencies_file_to_string( &self, - target_path: &Path, + target_path: &EscapedPath, ) -> Result { let mut s = Vec::new(); @@ -311,8 +304,8 @@ impl LinkerWriter<'_> { Ok(()) } - pub fn export_symbol_header_to_file(&self, path: &Path) -> Result<(), SlinkyError> { - let mut f = utils::create_file_and_parents(path)?; + pub fn export_symbol_header_to_file(&self, path: &EscapedPath) -> Result<(), SlinkyError> { + let mut f = utils::create_file_and_parents(path.as_ref())?; self.export_symbol_header(&mut f) } @@ -756,7 +749,7 @@ impl LinkerWriter<'_> { segment: &Segment, section: &str, sections: &[String], - base_path: &Path, + base_path: &EscapedPath, ) -> Result<(), SlinkyError> { if !self.rs.should_emit_entry( &file.exclude_if_any, @@ -774,36 +767,28 @@ impl LinkerWriter<'_> { // TODO: figure out glob support match file.kind { FileKind::Object => { - let mut path = base_path.to_path_buf(); - path.extend(&file.path); + let mut path = base_path.clone(); + path.push(file.path_escaped(self.rs)?); - let escaped_path = self.rs.escape_path(&path)?; - - self.buffer.writeln(&format!( - "{}({}{});", - escaped_path.display(), - section, - wildcard - )); - if !self.files_paths.contains(&escaped_path) { - self.files_paths.insert(escaped_path); + self.buffer + .writeln(&format!("{}({}{});", path.display(), section, wildcard)); + if !self.files_paths.contains(&path) { + self.files_paths.insert(path); } } FileKind::Archive => { - let mut path = base_path.to_path_buf(); - path.extend(&file.path); - - let escaped_path = self.rs.escape_path(&path)?; + let mut path = base_path.clone(); + path.push(file.path_escaped(self.rs)?); self.buffer.writeln(&format!( "{}:{}({}{});", - escaped_path.display(), + path.display(), file.subfile, section, wildcard )); - if !self.files_paths.contains(&escaped_path) { - self.files_paths.insert(escaped_path); + if !self.files_paths.contains(&path) { + self.files_paths.insert(path); } } FileKind::Pad => { @@ -819,9 +804,9 @@ impl LinkerWriter<'_> { } } FileKind::Group => { - let mut new_base_path = base_path.to_path_buf(); + let mut new_base_path = base_path.clone(); - new_base_path.extend(&file.dir); + new_base_path.push(file.dir_escaped(self.rs)?); for file_of_group in &file.files { //self.emit_file(file_of_group, segment, section, sections, &new_base_path)?; @@ -845,7 +830,7 @@ impl LinkerWriter<'_> { segment: &Segment, section: &str, sections: &[String], - base_path: &Path, + base_path: &EscapedPath, ) -> Result<(), SlinkyError> { if !file.section_order.is_empty() { // Keys specify the section and value specify where it will be put. @@ -886,11 +871,10 @@ impl LinkerWriter<'_> { section: &str, sections: &[String], ) -> Result<(), SlinkyError> { - let mut base_path = PathBuf::new(); + let mut base_path = self.d.settings.base_path_escaped(self.rs)?; - base_path.extend(&self.d.settings.base_path); if !self.reference_partial_objects { - base_path.extend(&segment.dir); + base_path.push(segment.dir_escaped(self.rs)?); } for file in &segment.files { diff --git a/slinky/src/partial_linker_writer.rs b/slinky/src/partial_linker_writer.rs index 59fe07e..81f17d8 100644 --- a/slinky/src/partial_linker_writer.rs +++ b/slinky/src/partial_linker_writer.rs @@ -1,11 +1,9 @@ /* SPDX-FileCopyrightText: © 2024 decompals */ /* SPDX-License-Identifier: MIT */ -use std::path::{Path, PathBuf}; - use crate::{ - Document, FileInfo, LinkerWriter, RuntimeSettings, ScriptExporter, ScriptGenerator, - ScriptImporter, Segment, SlinkyError, SymbolAssignment, + Document, EscapedPath, FileInfo, LinkerWriter, RuntimeSettings, ScriptExporter, + ScriptGenerator, ScriptImporter, Segment, SlinkyError, SymbolAssignment, }; pub struct PartialLinkerWriter<'a> { @@ -55,13 +53,8 @@ impl ScriptImporter for PartialLinkerWriter<'_> { self.partial_writers .push((partial_writer, segment.name.clone())); - let mut p = PathBuf::new(); + let mut p = self.d.settings.partial_build_segments_folder.clone(); - p.push( - &self - .rs - .escape_path(&self.d.settings.partial_build_segments_folder)?, - ); p.push(&format!("{}.o", segment.name)); self.main_writer @@ -83,18 +76,13 @@ impl ScriptImporter for PartialLinkerWriter<'_> { } impl ScriptExporter for PartialLinkerWriter<'_> { - fn export_linker_script_to_file(&self, path: &Path) -> Result<(), SlinkyError> { + fn export_linker_script_to_file(&self, path: &EscapedPath) -> Result<(), SlinkyError> { self.main_writer.export_linker_script_to_file(path)?; for (partial, name) in &self.partial_writers { - let mut p = PathBuf::new(); + let mut p = self.d.settings.partial_scripts_folder_escaped(self.rs)?; - p.push( - &self - .rs - .escape_path(&self.d.settings.partial_scripts_folder)?, - ); - p.push(&format!("{}.ld", name)); + p.push(EscapedPath::from(format!("{}.ld", name))); partial.export_linker_script_to_file(&p)?; } @@ -119,24 +107,18 @@ impl ScriptExporter for PartialLinkerWriter<'_> { if self.d.settings.d_path.is_some() { for (partial, name) in &self.partial_writers { - let mut target_path = PathBuf::new(); + let mut target_path = self.d.settings.base_path_escaped(self.rs)?; - target_path.push(&self.rs.escape_path(&self.d.settings.base_path)?); target_path.push( - &self - .rs - .escape_path(&self.d.settings.partial_build_segments_folder)?, + self.d + .settings + .partial_build_segments_folder_escaped(self.rs)?, ); - target_path.push(&format!("{}.o", name)); + target_path.push(EscapedPath::from(format!("{}.o", name))); - let mut d_path = PathBuf::new(); + let mut d_path = self.d.settings.partial_scripts_folder_escaped(self.rs)?; - d_path.push( - &self - .rs - .escape_path(&self.d.settings.partial_scripts_folder)?, - ); - d_path.push(&format!("{}.d", name)); + d_path.push(EscapedPath::from(format!("{}.d", name))); partial.export_dependencies_file_to_file(&d_path, &target_path)?; } diff --git a/slinky/src/runtime_settings.rs b/slinky/src/runtime_settings.rs index 5fc3e64..5afb122 100644 --- a/slinky/src/runtime_settings.rs +++ b/slinky/src/runtime_settings.rs @@ -3,10 +3,11 @@ use std::{ collections::HashMap, + fmt::Debug, path::{Path, PathBuf}, }; -use crate::SlinkyError; +use crate::{EscapedPath, SlinkyError}; #[derive(PartialEq, Debug)] pub struct RuntimeSettings { @@ -68,7 +69,7 @@ impl RuntimeSettings { /// Replace all the `{key}` instances on the `path` argument with the corresponding value specified on the global `custom_options`. /// /// If the `key` is not present on the custom options then it returns an error. - pub fn escape_path(&self, path: &Path) -> Result { + pub fn escape_path(&self, path: &Path) -> Result { let mut new_path = PathBuf::new(); for component in path.iter() { @@ -118,7 +119,7 @@ impl RuntimeSettings { } } - Ok(new_path) + Ok(EscapedPath(new_path)) } pub fn should_emit_entry( diff --git a/slinky/src/segment.rs b/slinky/src/segment.rs index cf54ba9..7698422 100644 --- a/slinky/src/segment.rs +++ b/slinky/src/segment.rs @@ -8,7 +8,7 @@ use serde::Deserialize; use crate::{ absent_nullable::AbsentNullable, file_info::{FileInfo, FileInfoSerial}, - Settings, SlinkyError, + EscapedPath, RuntimeSettings, Settings, SlinkyError, }; #[derive(PartialEq, Debug, Clone)] @@ -87,6 +87,10 @@ impl Segment { fill_value: self.fill_value, } } + + pub fn dir_escaped(&self, rs: &RuntimeSettings) -> Result { + rs.escape_path(&self.dir) + } } #[derive(Deserialize, PartialEq, Debug)] diff --git a/slinky/src/settings.rs b/slinky/src/settings.rs index 9d4348b..e254b40 100644 --- a/slinky/src/settings.rs +++ b/slinky/src/settings.rs @@ -5,7 +5,8 @@ use serde::Deserialize; use std::{collections::HashMap, path::PathBuf}; use crate::{ - absent_nullable::AbsentNullable, linker_symbols_style::LinkerSymbolsStyle, SlinkyError, + absent_nullable::AbsentNullable, linker_symbols_style::LinkerSymbolsStyle, EscapedPath, + RuntimeSettings, SlinkyError, }; #[derive(PartialEq, Debug)] @@ -213,6 +214,53 @@ impl Default for Settings { } } +impl Settings { + pub fn base_path_escaped(&self, rs: &RuntimeSettings) -> Result { + rs.escape_path(&self.base_path) + } + + pub fn d_path_escaped(&self, rs: &RuntimeSettings) -> Result, SlinkyError> { + match &self.d_path { + Some(p) => Ok(Some(rs.escape_path(p)?)), + None => Ok(None), + } + } + + pub fn target_path_escaped( + &self, + rs: &RuntimeSettings, + ) -> Result, SlinkyError> { + match &self.target_path { + Some(p) => Ok(Some(rs.escape_path(p)?)), + None => Ok(None), + } + } + + pub fn symbols_header_path_escaped( + &self, + rs: &RuntimeSettings, + ) -> Result, SlinkyError> { + match &self.symbols_header_path { + Some(p) => Ok(Some(rs.escape_path(p)?)), + None => Ok(None), + } + } + + pub fn partial_scripts_folder_escaped( + &self, + rs: &RuntimeSettings, + ) -> Result { + rs.escape_path(&self.partial_scripts_folder) + } + + pub fn partial_build_segments_folder_escaped( + &self, + rs: &RuntimeSettings, + ) -> Result { + rs.escape_path(&self.partial_build_segments_folder) + } +} + #[derive(Deserialize, PartialEq, Debug)] #[serde(deny_unknown_fields)] pub(crate) struct SettingsSerial { diff --git a/slinky/src/traits.rs b/slinky/src/traits.rs index 4c958dd..a0919dc 100644 --- a/slinky/src/traits.rs +++ b/slinky/src/traits.rs @@ -1,9 +1,7 @@ /* SPDX-FileCopyrightText: © 2024 decompals */ /* SPDX-License-Identifier: MIT */ -use std::path::Path; - -use crate::{Document, Segment, SlinkyError, SymbolAssignment}; +use crate::{Document, EscapedPath, Segment, SlinkyError, SymbolAssignment}; mod private { use crate::{LinkerWriter, PartialLinkerWriter}; @@ -30,7 +28,7 @@ pub trait ScriptImporter: private::Sealed { } pub trait ScriptExporter: private::Sealed { - fn export_linker_script_to_file(&self, path: &Path) -> Result<(), SlinkyError>; + fn export_linker_script_to_file(&self, path: &EscapedPath) -> Result<(), SlinkyError>; fn export_linker_script_to_string(&self) -> Result; fn save_other_files(&self) -> Result<(), SlinkyError>; diff --git a/slinky/tests/integration_test.rs b/slinky/tests/integration_test.rs index 430b102..9ef16f7 100644 --- a/slinky/tests/integration_test.rs +++ b/slinky/tests/integration_test.rs @@ -92,7 +92,7 @@ fn check_d_generation(yaml_path: &Path, ld_path: &Path) -> Result<(), SlinkyErro let expected_d_contents = fs::read_to_string(ld_path).expect("unable to read expected d file"); - let target_path = document.settings.target_path.as_ref().unwrap(); + let target_path = &document.settings.target_path_escaped(&rs)?.unwrap(); compare_multiline_strings( &expected_d_contents, &writer @@ -175,7 +175,9 @@ fn test_partial_linking_script_generation( p.push(".."); p.push( - rs.escape_path(&document.settings.partial_scripts_folder) + document + .settings + .partial_scripts_folder_escaped(&rs) .expect("Not able to escape path"), ); p.push(&format!("{}.ld", name)); @@ -192,6 +194,8 @@ fn test_partial_linking_script_generation( #[rstest] fn test_partial_linking_d_generation(#[files("../tests/partial_linking/*.d")] d_path: PathBuf) { + use slinky::EscapedPath; + let yaml_path = d_path.with_extension("yaml"); let document = slinky::Document::read_file(&yaml_path).expect("unable to read original file"); @@ -202,7 +206,7 @@ fn test_partial_linking_d_generation(#[files("../tests/partial_linking/*.d")] d_ let expected_d_contents = fs::read_to_string(d_path).expect("unable to read expected d file"); - let target_path = document.settings.target_path.as_ref().unwrap(); + let target_path = &document.settings.target_path_escaped(&rs).unwrap().unwrap(); compare_multiline_strings( &expected_d_contents, &writer @@ -216,7 +220,9 @@ fn test_partial_linking_d_generation(#[files("../tests/partial_linking/*.d")] d_ p.push(".."); p.push( - rs.escape_path(&document.settings.partial_scripts_folder) + document + .settings + .partial_scripts_folder_escaped(&rs) .expect("Unable to escape path"), ); p.push(&format!("{}.d", name)); @@ -224,16 +230,19 @@ fn test_partial_linking_d_generation(#[files("../tests/partial_linking/*.d")] d_ let expected_partial_ld_contents = fs::read_to_string(p).expect("unable to read expected d file"); - let mut partial_target = PathBuf::new(); - partial_target.push( - rs.escape_path(&document.settings.base_path) - .expect("Failed to escape path"), - ); + let mut partial_target = document + .settings + .base_path_escaped(&rs) + .expect("Failed to escape path"); + partial_target.push( - rs.escape_path(&document.settings.partial_build_segments_folder) + document + .settings + .partial_build_segments_folder_escaped(&rs) .expect("Failed to escape path"), ); - partial_target.push(&format!("{}.o", name)); + partial_target.push(EscapedPath::from(format!("{}.o", name))); + compare_multiline_strings( &expected_partial_ld_contents, &partial