janezicmatej/ahab
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: keep a path's bytes, so a filename that is not utf-8 survives

Browse Source
This commit is contained in:
Matej Janezic
2026-09-09 12:01:59 +00:00
parent b4b6d5918f
commit e146b0460a
8 changed files with 172 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/cmd/argv.rs
View File

@@ -1,4 +1,5 @@
use std::{
ffi::{OsStr, OsString},
fmt::Display,
fs::File,
os::unix::process::CommandExt,
@@ -13,7 +14,7 @@ use crate::output::write_err;
// a program and its arguments: the only thing in ahab that knows what argv looks like
#[derive(Default, Clone)]
pub struct Argv(Vec<String>);
pub struct Argv(Vec<OsString>);
impl Display for Argv {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -22,42 +23,57 @@ impl Display for Argv {
}
impl Argv {
pub fn new(program: &str) -> Self {
Self(vec![program.to_string()])
pub fn new(program: impl AsRef<OsStr>) -> Self {
Self(vec![program.as_ref().to_owned()])
}
pub fn arg(mut self, arg: impl AsRef<str>) -> Self {
self.0.push(arg.as_ref().to_string());
pub fn arg(mut self, arg: impl AsRef<OsStr>) -> Self {
self.0.push(arg.as_ref().to_owned());
self
}
pub fn args<I, S>(mut self, args: I) -> Self
where
I: IntoIterator<Item = S>,
S: AsRef<str>,
S: AsRef<OsStr>,
{
self.0
.extend(args.into_iter().map(|arg| arg.as_ref().to_string()));
.extend(args.into_iter().map(|arg| arg.as_ref().to_owned()));
self
}
// a long option and the value it takes
pub fn flag(self, name: &str, value: impl AsRef<str>) -> Self {
pub fn flag(self, name: &str, value: impl AsRef<OsStr>) -> Self {
self.arg(name).arg(value)
}
pub fn program(&self) -> &str {
self.0.first().map(String::as_str).unwrap_or_default()
// for a message that names the tool being run, so lossy is what is wanted
pub fn program(&self) -> String {
self.0
.first()
.map(|word| word.to_string_lossy().into_owned())
.unwrap_or_default()
}
pub fn words(&self) -> &[String] {
pub fn words(&self) -> &[OsString] {
&self.0
}
// one word list for `sh -c`, quoted so a value with a space survives the shell
// one word list for `sh -c`, quoted so a value with a space survives the
// shell. lossy: this is for reading, and for scripts built only out of the
// ascii and compose-derived words that reach a pipeline
pub fn quoted(&self) -> String {
// a nul byte is the only thing shlex refuses, and no argv can hold one
shlex::try_join(self.0.iter().map(String::as_str)).unwrap_or_default()
let words: Vec<String> = self
.0
.iter()
.map(|word| word.to_string_lossy().into_owned())
.collect();
// a nul byte is the only thing shlex refuses, and no argv can hold one,
// so this only happens for a word that could never have been run. it
// still has to be readable: an empty string would name no command at
// all in the very message explaining what went wrong
shlex::try_join(words.iter().map(String::as_str)).unwrap_or_else(|_| words.join(" "))
}
pub fn run(&self, ctx: &Ctx) -> Result<()> {
@@ -72,6 +88,14 @@ impl Argv {
// reading changes nothing, so a dry run answers the question for real
pub fn capture(&self, ctx: &Ctx) -> Result<String> {
let stdout = self.capture_bytes(ctx)?;
String::from_utf8(stdout).with_context(|| format!("reading the output of `{self}`"))
}
// git's -z listings are raw bytes: a path on unix is bytes, not text, and
// one that is not utf-8 would otherwise fail the whole listing it appears in
pub fn capture_bytes(&self, ctx: &Ctx) -> Result<Vec<u8>> {
let out = self.command(ctx)?.output()?;
if !out.status.success() {
@@ -85,7 +109,7 @@ impl Argv {
});
}
Ok(String::from_utf8(out.stdout)?)
Ok(out.stdout)
}
// replaces this process, so the command's exit code and signals become ours

22
src/cmd/docker.rs
View File

@@ -1,3 +1,6 @@
use std::ffi::OsString;
use std::path::Path;
use super::{Argv, Cmd};
// docker exec, around a command that runs inside the container
@@ -47,24 +50,25 @@ impl Cmd for Exec {
}
}
// docker cp, in either direction
// docker cp, in either direction. the local side stays an OsString: it is a
// path the user named, and a lossy one would copy something else
pub struct Cp {
from: String,
to: String,
from: OsString,
to: OsString,
}
impl Cp {
pub fn into_container(local: &str, container: &str, remote: &str) -> Self {
pub fn into_container(local: &Path, container: &str, remote: &str) -> Self {
Self {
from: local.to_string(),
to: format!("{container}:{remote}"),
from: local.as_os_str().to_owned(),
to: OsString::from(format!("{container}:{remote}")),
}
}
pub fn out_of_container(container: &str, remote: &str, local: &str) -> Self {
pub fn out_of_container(container: &str, remote: &str, local: &Path) -> Self {
Self {
from: format!("{container}:{remote}"),
to: local.to_string(),
from: OsString::from(format!("{container}:{remote}")),
to: local.as_os_str().to_owned(),
}
}
}

49
src/cmd/git.rs
View File

@@ -1,10 +1,22 @@
use std::ffi::OsString;
use std::path::Path;
use super::{Argv, Cmd};
// -C has no long form; it runs git as though from that directory
fn git(root: &Path) -> Argv {
Argv::new("git").arg("-C").arg(root.to_string_lossy())
Argv::new("git").arg("-C").arg(root)
}
// `--` stops git reading a leading dash as an option, but pathspec magic is
// read after it too, so a file actually named `:(exclude)x` or `:!x` would ask
// about a different set of paths than the one in hand. built as an OsString: a
// path is bytes, and a lossy one asks git about a name nothing on disk has
fn literal(path: &Path) -> OsString {
let mut spec = OsString::from(":(literal)");
spec.push(path);
spec
}
// the root of the repository the working directory is in
@@ -31,7 +43,7 @@ pub struct LsFiles<'a> {
root: &'a Path,
tracked: bool,
ignored: bool,
pathspecs: Vec<String>,
pathspecs: Vec<OsString>,
}
impl<'a> LsFiles<'a> {
@@ -64,7 +76,7 @@ impl<'a> LsFiles<'a> {
pub fn limited_to<P: AsRef<Path>>(mut self, pathspecs: &[P]) -> Self {
self.pathspecs = pathspecs
.iter()
.map(|path| path.as_ref().to_string_lossy().to_string())
.map(|path| literal(path.as_ref()))
.collect();
self
}
@@ -72,15 +84,14 @@ impl<'a> LsFiles<'a> {
impl Cmd for LsFiles<'_> {
fn argv(&self) -> Argv {
let mut argv = git(self.root).arg("ls-files");
// -z has no long form; it separates the paths with NUL, which is the
// only separator a filename cannot contain
let mut argv = git(self.root).arg("ls-files").arg("-z");
if self.tracked {
argv = argv.arg("--cached");
} else {
// -z has no long form; it separates the paths with NUL, which is the
// only separator a filename cannot contain
argv = argv
.arg("-z")
.arg("--others")
.arg("--exclude-standard")
.arg("--directory")
@@ -113,7 +124,7 @@ impl Cmd for CheckIgnore<'_> {
.arg("check-ignore")
.arg("--quiet")
.arg("--")
.arg(self.path.to_string_lossy())
.arg(self.path)
}
}
@@ -134,7 +145,7 @@ mod tests {
assert_eq!(
argv.quoted(),
"git -C /repo ls-files -z --others --exclude-standard --directory \
--no-empty-directory --ignored -- 'a b'"
--no-empty-directory --ignored -- ':(literal)a b'",
);
}
@@ -144,7 +155,25 @@ mod tests {
.limited_to(&[PathBuf::from(".env")])
.argv();
assert_eq!(argv.quoted(), "git -C /repo ls-files --cached -- .env");
assert_eq!(
argv.quoted(),
"git -C /repo ls-files -z --cached -- ':(literal).env'"
);
}
#[test]
fn a_pathspec_is_asked_for_literally_however_it_is_spelled() {
// git reads magic after `--` too, so these names must not become one
for name in [":(exclude)secret", ":!secret", ":(glob)**"] {
let argv = LsFiles::tracked(Path::new("/repo"))
.limited_to(&[PathBuf::from(name)])
.argv();
assert_eq!(
argv.words().last().unwrap(),
format!(":(literal){name}").as_str()
);
}
}
#[test]

4
src/cmd/mod.rs
View File

@@ -55,6 +55,10 @@ pub trait Cmd {
self.argv().capture(ctx)
}
fn capture_bytes(&self, ctx: &Ctx) -> Result<Vec<u8>> {
self.argv().capture_bytes(ctx)
}
fn replace(&self, ctx: &Ctx) -> Result<()> {
self.argv().replace(ctx)
}

81
src/commands/link/check.rs
View File

@@ -1,4 +1,6 @@
use fs_err::{read_dir, read_link};
use std::ffi::OsString;
use std::os::unix::ffi::{OsStrExt, OsStringExt};
use std::path::{Path, PathBuf};
use anyhow::{Result, anyhow};
@@ -6,7 +8,7 @@ use anyhow::{Result, anyhow};
use super::store::{Repo, resolve, symlink_metadata_opt};
use crate::cmd::{Cmd, LsFiles};
use crate::ctx::Ctx;
use crate::output::{line, text};
use crate::output::{line, text, write_bytes};
// whether anything is outside the store, which main turns into an exit code
pub fn check(
@@ -23,13 +25,10 @@ pub fn check(
// git lists untracked and ignored separately
for (mark, ignored) in [(UNTRACKED, false), (IGNORED, true)] {
for entry in list_others(ctx, &repo, ignored, &pathspecs)? {
let rel = PathBuf::from(entry.trim_end_matches('/'));
// --directory collapses a wholly untracked dir into `dir/`
if entry.ends_with('/') {
exposed.extend(walk(&repo, &rel, mark)?.1);
} else {
exposed.extend(classify(&repo, &rel, mark)?);
match entry.strip_suffix(b"/") {
Some(dir) => exposed.extend(walk(&repo, &path_from(dir), mark)?.1),
None => exposed.extend(classify(&repo, &path_from(&entry), mark)?),
}
}
}
@@ -46,19 +45,36 @@ pub fn check(
}
fn print_porcelain(exposed: &[Exposed], null: bool) {
let end = if null { '\0' } else { '\n' };
// a filename can hold an arrow but not a NUL, as `git status -z` also assumes
let between = if null { "\0" } else { " -> " };
// -z is the format for scripts that must survive any filename, so its
// records are written as the bytes a path actually is. a filename can hold
// an arrow but not a NUL, as `git status -z` also assumes
if null {
for item in exposed {
let mut record = item.code().into_bytes();
record.push(b' ');
record.extend_from_slice(item.name.as_os_str().as_bytes());
if let Some(dest) = &item.dest {
record.push(0);
record.extend_from_slice(dest.as_os_str().as_bytes());
}
record.push(0);
write_bytes(&record);
}
return;
}
for item in exposed {
match &item.dest {
Some(dest) => text!(
"{} {}{between}{}{end}",
"{} {} -> {}\n",
item.code(),
item.name,
item.name.display(),
dest.display()
),
None => text!("{} {}{end}", item.code(), item.name),
None => text!("{} {}\n", item.code(), item.name.display()),
}
}
}
@@ -98,8 +114,8 @@ fn print_listing(repo: &Repo, exposed: &[Exposed]) {
line!("\n{heading}\n{hint}");
for item in items {
match &item.dest {
Some(dest) => line!("\t{} -> {}", item.name, dest.display()),
None => line!("\t{}", item.name),
Some(dest) => line!("\t{} -> {}", item.name.display(), dest.display()),
None => line!("\t{}", item.name.display()),
}
}
}
@@ -126,12 +142,12 @@ const ELSEWHERE: char = '>';
struct Exposed {
mark: char,
name: String,
name: PathBuf,
dest: Option<PathBuf>,
}
impl Exposed {
fn content(mark: char, name: String) -> Self {
fn content(mark: char, name: PathBuf) -> Self {
Self {
mark,
name,
@@ -151,7 +167,7 @@ impl Exposed {
fn classify(repo: &Repo, rel: &Path, mark: char) -> Result<Option<Exposed>> {
let src = repo.root.join(rel);
let name = rel.display().to_string();
let name = rel.to_path_buf();
let Some(meta) = symlink_metadata_opt(&src)? else {
return Ok(None);
@@ -196,8 +212,10 @@ fn walk(repo: &Repo, rel: &Path, mark: char) -> Result<(usize, Vec<Exposed>)> {
// nothing below is in the store, so collapse to one line
if handled == 0 && !exposed.is_empty() {
let name = format!("{}/", rel.display());
return Ok((0, vec![Exposed::content(mark, name)]));
let mut name = rel.as_os_str().to_owned();
name.push("/");
return Ok((0, vec![Exposed::content(mark, PathBuf::from(name))]));
}
Ok((handled, exposed))
@@ -230,16 +248,25 @@ fn list_others(
repo: &Repo,
ignored: bool,
pathspecs: &[PathBuf],
) -> Result<Vec<String>> {
) -> Result<Vec<Vec<u8>>> {
let mut listing = LsFiles::untracked(&repo.root).limited_to(pathspecs);
if ignored {
listing = listing.ignored();
}
Ok(listing
.capture(ctx)?
.split('\0')
.filter(|p| !p.is_empty())
.map(String::from)
.collect())
Ok(split_nul(&listing.capture_bytes(ctx)?))
}
// every listing check reads is asked for with -z, and kept as the bytes git
// wrote: a path is not obliged to be utf-8, and one that is not used to fail
// the whole listing rather than the one entry
fn split_nul(out: &[u8]) -> Vec<Vec<u8>> {
out.split(|byte| *byte == 0)
.filter(|record| !record.is_empty())
.map(<[u8]>::to_vec)
.collect()
}
fn path_from(bytes: &[u8]) -> PathBuf {
PathBuf::from(OsString::from_vec(bytes.to_vec()))
}

19
src/commands/link/store.rs
View File

@@ -1,6 +1,7 @@
use fs_err as fs;
use std::env;
use std::ffi::OsString;
use std::os::unix::ffi::OsStringExt;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result, anyhow};
@@ -69,16 +70,21 @@ pub(super) fn resolve(path: &Path) -> Result<PathBuf> {
}
fn git_root(ctx: &Ctx) -> Result<PathBuf> {
// bytes, since the checkout can live at a path that is not utf-8, and with
// the cause kept: git not being installed and the directory not being a
// repository are different problems with the same one-line answer otherwise
let root = RevParse
.capture(ctx)
.map_err(|_| anyhow!("not inside a git repository"))?;
.capture_bytes(ctx)
.context("asking git for the repository root")?;
let root = root.trim().to_string();
let root = root.strip_suffix(b"\n").unwrap_or(&root);
if root.is_empty() {
return Err(anyhow!("git reported an empty repository root"));
}
Ok(fs::canonicalize(&root)?)
Ok(fs::canonicalize(PathBuf::from(OsString::from_vec(
root.to_vec(),
)))?)
}
fn repo_components(ctx: &Ctx, root: &Path) -> Result<PathBuf> {
@@ -170,9 +176,12 @@ fn non_empty_var(name: &str) -> Option<OsString> {
}
pub(super) fn tracked(ctx: &Ctx, repo: &Repo, rel: &Path) -> Result<bool> {
// bytes: the listing echoes back the path asked about, which is not obliged
// to be utf-8, and failing to read it would refuse the path for the wrong
// reason rather than answering whether git tracks it
let listed = LsFiles::tracked(&repo.root)
.limited_to(&[rel])
.capture(ctx)?;
.capture_bytes(ctx)?;
Ok(!listed.is_empty())
}

8
src/commands/postgres.rs
View File

@@ -81,11 +81,7 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
// a directory cannot be streamed, so it is the one shape that gets copied in
if matches!(dump, Dump::Directory) {
when_ready(
ctx,
&db,
&Cp::into_container(&file.to_string_lossy(), &db.container, &remote),
)?;
when_ready(ctx, &db, &Cp::into_container(file, &db.container, &remote))?;
}
let kind = match &dump {
@@ -284,7 +280,7 @@ fn dump_directory(ctx: &Ctx, db: &Database, target: &Path) -> Result<()> {
.in_container(&db.container)
.run(ctx)?;
let copied = Cp::out_of_container(&db.container, &remote, &target.to_string_lossy()).run(ctx);
let copied = Cp::out_of_container(&db.container, &remote, target).run(ctx);
let _ = Rm::recursive(&remote).in_container(&db.container).run(ctx);

7
src/output.rs
View File

@@ -53,6 +53,13 @@ pub(crate) fn write_text(args: Arguments) {
}
}
// a path on unix is bytes, not text, so a record naming one is written as bytes
pub(crate) fn write_bytes(bytes: &[u8]) {
if writable() {
finish(io::stdout().write_all(bytes));
}
}
// stderr is commentary about the work, not the work itself. there is nowhere to
// report that it could not be written, so the error goes nowhere -- eprintln!
// panics instead, which turns a closed pipe into a crash