Skip to content
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

Quote launch process command paths #239

Merged
merged 8 commits into from
Mar 19, 2025
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
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions buildpacks/dotnet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- The buildpack now sanitizes launch process type names, based on project assembly names, by filtering out invalid characters. ([#237](https://github.com/heroku/buildpacks-dotnet/pull/237))
- Launch process commands with paths containing special characters (including spaces) are now properly quoted. ([#239](https://github.com/heroku/buildpacks-dotnet/pull/239))

## [0.3.5] - 2025-03-19

Expand Down
1 change: 1 addition & 0 deletions buildpacks/dotnet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ semver = "1.0"
serde = "1"
serde_json = "1"
sha2 = "0.10"
shell-words = "1.1.0"

[dev-dependencies]
libcnb-test = "0.28"
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/dotnet/src/dotnet/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct Metadata {
assembly_name: Option<String>,
}

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Clone, Copy)]
pub(crate) enum ProjectType {
ConsoleApplication,
WebApplication,
Expand Down
143 changes: 86 additions & 57 deletions buildpacks/dotnet/src/launch_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::dotnet::project::ProjectType;
use crate::dotnet::solution::Solution;
use crate::Project;
use libcnb::data::launch::{Process, ProcessBuilder, ProcessType};
use std::path::PathBuf;
use std::path::{Path, PathBuf};

/// Detects processes in a solution's projects
pub(crate) fn detect_solution_processes(solution: &Solution) -> Vec<Process> {
Expand All @@ -23,23 +23,36 @@ fn project_launch_process(solution: &Solution, project: &Project) -> Option<Proc
}
let relative_executable_path = relative_executable_path(solution, project);

let command = build_command(&relative_executable_path, project.project_type);

Some(ProcessBuilder::new(project_process_type(project), ["bash", "-c", &command]).build())
}

/// Constructs the shell command for launching the process
fn build_command(relative_executable_path: &Path, project_type: ProjectType) -> String {
let parent_dir = relative_executable_path
.parent()
.expect("Executable path should always have a parent directory")
.to_str()
.expect("Path should be valid UTF-8");

let file_name = relative_executable_path
.file_name()
.expect("Executable path should always have a file name")
.to_str()
.expect("Path should be valid UTF-8");

let mut command = format!(
"cd {}; ./{}",
relative_executable_path
.parent()
.expect("Path to always have a parent directory")
.display(),
relative_executable_path
.file_name()
.expect("Path to never terminate in `..`")
.to_string_lossy()
shell_words::quote(parent_dir),
shell_words::quote(file_name)
);

if project.project_type == ProjectType::WebApplication {
if project_type == ProjectType::WebApplication {
command.push_str(" --urls http://*:$PORT");
}

Some(ProcessBuilder::new(project_process_type(project), ["bash", "-c", &command]).build())
command
}

/// Returns a sanitized process type name, ensuring it is always valid
Expand All @@ -56,9 +69,9 @@ fn relative_executable_path(solution: &Solution, project: &Project) -> PathBuf {
solution
.path
.parent()
.expect("Solution path to have a parent"),
.expect("Solution file should be in a directory"),
)
.expect("Project to be nested in solution parent directory")
.expect("Executable path should inside the solution's directory")
.to_path_buf()
}

Expand Down Expand Up @@ -88,16 +101,24 @@ mod tests {
use libcnb::data::process_type;
use std::path::PathBuf;

fn create_test_project(path: &str, assembly_name: &str, project_type: ProjectType) -> Project {
Project {
path: PathBuf::from(path),
target_framework: "net9.0".to_string(),
project_type,
assembly_name: assembly_name.to_string(),
}
}

#[test]
fn test_detect_solution_processes_web_app() {
let solution = Solution {
path: PathBuf::from("/tmp/foo.sln"),
projects: vec![Project {
path: PathBuf::from("/tmp/bar/bar.csproj"),
target_framework: "net9.0".to_string(),
project_type: ProjectType::WebApplication,
assembly_name: "bar".to_string(),
}],
projects: vec![create_test_project(
"/tmp/bar/bar.csproj",
"bar",
ProjectType::WebApplication,
)],
};

let expected_processes = vec![Process {
Expand All @@ -116,23 +137,22 @@ mod tests {
}

#[test]
fn test_detect_solution_processes_console_app() {
fn test_detect_solution_processes_with_spaces() {
let solution = Solution {
path: PathBuf::from("/tmp/foo.sln"),
projects: vec![Project {
path: PathBuf::from("/tmp/bar/bar.csproj"),
target_framework: "net9.0".to_string(),
project_type: ProjectType::ConsoleApplication,
assembly_name: "bar".to_string(),
}],
path: PathBuf::from("/tmp/My Solution With Spaces.sln"),
projects: vec![create_test_project(
"/tmp/My Project With Spaces/project.csproj",
"My App",
ProjectType::ConsoleApplication,
)],
};

let expected_processes = vec![Process {
r#type: process_type!("bar"),
r#type: process_type!("MyApp"),
command: vec![
"bash".to_string(),
"-c".to_string(),
"cd bar/bin/publish; ./bar".to_string(),
"cd 'My Project With Spaces/bin/publish'; ./'My App'".to_string(),
],
args: vec![],
default: false,
Expand All @@ -143,28 +163,31 @@ mod tests {
}

#[test]
fn test_project_launch_process_non_executable() {
fn test_relative_executable_path() {
let solution = Solution {
path: PathBuf::from("/tmp/foo.sln"),
projects: vec![Project {
path: PathBuf::from("/tmp/bar/bar.csproj"),
target_framework: "net9.0".to_string(),
project_type: ProjectType::Unknown,
assembly_name: "bar".to_string(),
}],
path: PathBuf::from("/tmp/solution.sln"),
projects: vec![],
};

assert!(detect_solution_processes(&solution).is_empty());
let project = create_test_project(
"/tmp/project/project.csproj",
"TestApp",
ProjectType::ConsoleApplication,
);

assert_eq!(
relative_executable_path(&solution, &project),
PathBuf::from("project/bin/publish/TestApp")
);
}

#[test]
fn test_project_executable_path() {
let project = Project {
path: PathBuf::from("/tmp/project/project.csproj"),
target_framework: "net9.0".to_string(),
project_type: ProjectType::ConsoleApplication,
assembly_name: "TestApp".to_string(),
};
let project = create_test_project(
"/tmp/project/project.csproj",
"TestApp",
ProjectType::ConsoleApplication,
);

assert_eq!(
project_executable_path(&project),
Expand All @@ -173,22 +196,28 @@ mod tests {
}

#[test]
fn test_relative_executable_path() {
let solution = Solution {
path: PathBuf::from("/tmp/solution.sln"),
projects: vec![],
};
fn test_build_command_with_spaces() {
let executable_path = PathBuf::from("some/project with spaces/bin/publish/My App");

let project = Project {
path: PathBuf::from("/tmp/project/project.csproj"),
target_framework: "net9.0".to_string(),
project_type: ProjectType::ConsoleApplication,
assembly_name: "TestApp".to_string(),
};
assert_eq!(
build_command(&executable_path, ProjectType::ConsoleApplication),
"cd 'some/project with spaces/bin/publish'; ./'My App'"
);

assert_eq!(
relative_executable_path(&solution, &project),
PathBuf::from("project/bin/publish/TestApp")
build_command(&executable_path, ProjectType::WebApplication),
"cd 'some/project with spaces/bin/publish'; ./'My App' --urls http://*:$PORT"
);
}

#[test]
fn test_build_command_with_special_chars() {
let executable_path =
PathBuf::from("some/project with #special$chars/bin/publish/My-App+v1.2_Release!");

assert_eq!(
build_command(&executable_path, ProjectType::ConsoleApplication),
"cd 'some/project with #special$chars/bin/publish'; ./My-App+v1.2_Release!"
);
}

Expand Down