openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-27 18:48:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(git-utils): parse Git boolean syntax consistently

Browse Source
This commit is contained in:
Chris Bookholt
2026-07-03 15:55:43 -07:00
parent e8e8178e3a
commit 6b40c77a0f
4 changed files with 235 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/git-utils/src/git_config.rs
+65
View File
@@ -1,6 +1,71 @@
use std::path::Component;
use std::path::Path;
pub(crate) fn parse_git_boolean(value: &[u8]) -> Option<bool> {
if value.eq_ignore_ascii_case(b"true")
|| value.eq_ignore_ascii_case(b"yes")
|| value.eq_ignore_ascii_case(b"on")
{
return Some(true);
}
if value.is_empty()
|| value.eq_ignore_ascii_case(b"false")
|| value.eq_ignore_ascii_case(b"no")
|| value.eq_ignore_ascii_case(b"off")
{
return Some(false);
}
// Git parses the remaining boolean spellings through `git_parse_int`: C
// base-0 syntax, an optional binary-unit suffix, and signed `int` bounds.
let value = std::str::from_utf8(value)
.ok()?
.trim_start_matches(|value: char| value.is_ascii_whitespace());
let (negative, unsigned) = match value.as_bytes().first() {
Some(b'-') => (true, &value[1..]),
Some(b'+') => (false, &value[1..]),
Some(_) => (false, value),
None => return None,
};
let (base, unsigned) = if unsigned.starts_with("0x") || unsigned.starts_with("0X") {
(16, &unsigned[2..])
} else if unsigned.starts_with('0') {
(8, unsigned)
} else {
(10, unsigned)
};
let digit_count = unsigned
.bytes()
.take_while(|byte| match base {
8 => matches!(byte, b'0'..=b'7'),
10 => byte.is_ascii_digit(),
16 => byte.is_ascii_hexdigit(),
_ => false,
})
.count();
if digit_count == 0 {
return None;
}
let (digits, suffix) = unsigned.split_at(digit_count);
let factor = if suffix.is_empty() {
1_i128
} else if suffix.eq_ignore_ascii_case("k") {
1024
} else if suffix.eq_ignore_ascii_case("m") {
1024 * 1024
} else if suffix.eq_ignore_ascii_case("g") {
1024 * 1024 * 1024
} else {
return None;
};
let magnitude = i128::from_str_radix(digits, base).ok()?;
let signed = if negative { -magnitude } else { magnitude };
let value = signed.checked_mul(factor)?;
(i128::from(i32::MIN)..=i128::from(i32::MAX))
.contains(&value)
.then_some(value != 0)
}
pub(crate) fn path_is_within(path: &Path, root: &Path) -> bool {
let mut path_components = path.components();
for root_component in root.components() {
codex-rs/git-utils/src/git_config_tests.rs
+64
View File
@@ -1,5 +1,69 @@
use super::*;
#[test]
fn git_boolean_accepts_native_git_spellings() {
for (value, expected) in [
(b"true".as_slice(), true),
(b"TRUE".as_slice(), true),
(b"yes".as_slice(), true),
(b"On".as_slice(), true),
(b"1".as_slice(), true),
(b"2".as_slice(), true),
(b"-1".as_slice(), true),
(b"+1".as_slice(), true),
(b"01".as_slice(), true),
(b"0x1".as_slice(), true),
(b"0Xf".as_slice(), true),
(b"010".as_slice(), true),
(b"1k".as_slice(), true),
(b"1M".as_slice(), true),
(b"1g".as_slice(), true),
(b" 1".as_slice(), true),
(b"2147483647".as_slice(), true),
(b"-2147483648".as_slice(), true),
(b"-2g".as_slice(), true),
(b"false".as_slice(), false),
(b"FALSE".as_slice(), false),
(b"no".as_slice(), false),
(b"Off".as_slice(), false),
(b"".as_slice(), false),
(b"0".as_slice(), false),
(b"-0".as_slice(), false),
(b"+0".as_slice(), false),
(b"00".as_slice(), false),
(b"0x0".as_slice(), false),
(b"0k".as_slice(), false),
] {
assert_eq!(parse_git_boolean(value), Some(expected), "value {value:?}");
}
}
#[test]
fn git_boolean_rejects_values_native_git_rejects() {
for value in [
b"08".as_slice(),
b"-08".as_slice(),
b"2147483648".as_slice(),
b"-2147483649".as_slice(),
b"9223372036854775807".as_slice(),
b"2g".as_slice(),
b"-3g".as_slice(),
b"0x".as_slice(),
b"0b1".as_slice(),
b"1kb".as_slice(),
b"1foo".as_slice(),
b"1_0".as_slice(),
b"1 ".as_slice(),
b" ".as_slice(),
b"+".as_slice(),
b"-".as_slice(),
b"not-a-bool".as_slice(),
b"\xff".as_slice(),
] {
assert_eq!(parse_git_boolean(value), None, "value {value:?}");
}
}
#[test]
fn path_containment_uses_component_boundaries() {
let root = Path::new("/repo/root");
codex-rs/git-utils/src/repository_authority/plain_config.rs
+22 -31
View File
@@ -4,6 +4,7 @@ use std::path::PathBuf;
use super::bytes_to_path;
use super::read_bounded_file;
use crate::git_config::parse_git_boolean;
#[derive(Debug, Eq, PartialEq)]
pub(crate) enum CommonConfigAuthority {
@@ -50,7 +51,7 @@ pub(crate) fn inspect_plain_common_config_authority(
return Ok(CommonConfigAuthority::Unproven);
}
let bare = match unique_explicit_value(&config, "core", "bare") {
Ok(Some(value)) => match explicit_boolean(value.as_ref()) {
Ok(Some(value)) => match parse_git_boolean(value.as_ref()) {
Some(value) => Some(value),
None => return Ok(CommonConfigAuthority::Unproven),
},
@@ -58,7 +59,7 @@ pub(crate) fn inspect_plain_common_config_authority(
Err(()) => return Ok(CommonConfigAuthority::Unproven),
};
match unique_explicit_value(&config, "extensions", "worktreeConfig") {
Ok(Some(value)) if explicit_boolean(value.as_ref()) == Some(false) => {}
Ok(Some(value)) if parse_git_boolean(value.as_ref()) == Some(false) => {}
Ok(None) => {}
Ok(Some(_)) | Err(()) => return Ok(CommonConfigAuthority::Unproven),
}
@@ -99,39 +100,29 @@ fn unique_explicit_value(
let Some(sections) = config.sections_by_name(section_name) else {
return Ok(None);
};
let mut occurrences = 0usize;
let mut explicit_values = Vec::new();
let mut occurrence = None;
for section in sections {
if section.header().subsection_name().is_some() {
return Err(());
}
occurrences += section
.value_names()
.filter(|name| {
let name: &str = name.as_ref();
name.eq_ignore_ascii_case(value_name)
})
.count();
explicit_values.extend(
section
.values(value_name)
.into_iter()
.map(|value| value.as_ref().to_vec()),
);
for name in section.value_names() {
let name: &str = name.as_ref();
if !name.eq_ignore_ascii_case(value_name) {
continue;
}
if occurrence.is_some() {
return Err(());
}
let value = section
.value_implicit(name)
.ok_or(())?
.map(|value| value.as_ref().to_vec());
occurrence = Some(value);
}
}
match (occurrences, explicit_values.len()) {
(0, 0) => Ok(None),
(1, 1) => Ok(explicit_values.pop()),
_ => Err(()),
}
}
fn explicit_boolean(value: &[u8]) -> Option<bool> {
if value.eq_ignore_ascii_case(b"true") {
Some(true)
} else if value.eq_ignore_ascii_case(b"false") {
Some(false)
} else {
None
match occurrence {
None => Ok(None),
Some(Some(value)) => Ok(Some(value)),
Some(None) => Err(()),
}
}
codex-rs/git-utils/src/repository_authority_tests.rs
+84 -5
View File
@@ -81,6 +81,36 @@ fn plain_common_bare_parser_matches_native_boolean_syntax_and_is_stricter_on_dup
CommonConfigAuthority::Bare,
Some(true),
),
(
"yes",
b"[core]\n\tbare = yes\n".as_slice(),
CommonConfigAuthority::Bare,
Some(true),
),
(
"on",
b"[core]\n\tbare = on\n".as_slice(),
CommonConfigAuthority::Bare,
Some(true),
),
(
"numeric true",
b"[core]\n\tbare = 1\n".as_slice(),
CommonConfigAuthority::Bare,
Some(true),
),
(
"hex true",
b"[core]\n\tbare = 0x1\n".as_slice(),
CommonConfigAuthority::Bare,
Some(true),
),
(
"suffixed true",
b"[core]\n\tbare = 1k\n".as_slice(),
CommonConfigAuthority::Bare,
Some(true),
),
(
"false",
b"[core]\n\tbare = false\n".as_slice(),
@@ -133,13 +163,13 @@ fn plain_common_bare_parser_matches_native_boolean_syntax_and_is_stricter_on_dup
}
#[test]
fn common_bare_proof_accepts_only_explicit_boolean_literals() {
fn common_bare_proof_rejects_invalid_git_boolean_values() {
for (name, body) in [
("empty", b"[core]\n\tbare =\n".as_slice()),
("yes", b"[core]\n\tbare = yes\n".as_slice()),
("on", b"[core]\n\tbare = on\n".as_slice()),
("one", b"[core]\n\tbare = 1\n".as_slice()),
("leading zero", b"[core]\n\tbare = 08\n".as_slice()),
(
"negative leading zero",
b"[core]\n\tbare = -08\n".as_slice(),
),
(
"positive overflow",
b"[core]\n\tbare = 2147483648\n".as_slice(),
@@ -152,6 +182,8 @@ fn common_bare_proof_accepts_only_explicit_boolean_literals() {
"i64 max",
b"[core]\n\tbare = 9223372036854775807\n".as_slice(),
),
("suffix overflow", b"[core]\n\tbare = 2g\n".as_slice()),
("trailing junk", b"[core]\n\tbare = 1foo\n".as_slice()),
] {
let common = write_common_config(body);
assert_eq!(
@@ -162,6 +194,34 @@ fn common_bare_proof_accepts_only_explicit_boolean_literals() {
}
}
#[test]
fn common_bare_proof_accepts_explicit_false_worktree_config_spellings() {
for value in ["false", "no", "off", "", "0", "-0", "+0", "00", "0x0"] {
let common = write_common_config(
format!("[core]\n\tbare = yes\n[extensions]\n\tworktreeConfig = {value}\n").as_bytes(),
);
assert_eq!(
inspect_plain_common_config_authority(common.path()).expect(value),
CommonConfigAuthority::Bare,
"authority result for worktreeConfig={value:?}"
);
}
}
#[test]
fn common_bare_proof_rejects_explicit_true_worktree_config_spellings() {
for value in ["true", "yes", "on", "1", "-1", "0x1", "1k"] {
let common = write_common_config(
format!("[core]\n\tbare = yes\n[extensions]\n\tworktreeConfig = {value}\n").as_bytes(),
);
assert_eq!(
inspect_plain_common_config_authority(common.path()).expect(value),
CommonConfigAuthority::Unproven,
"authority result for worktreeConfig={value:?}"
);
}
}
#[test]
fn common_bare_proof_rejects_includes_worktree_config_and_relative_worktree() {
let include_target = tempfile::NamedTempFile::new().expect("included config");
@@ -226,6 +286,25 @@ fn common_config_absolute_worktree_is_returned_as_authority() {
);
}
#[test]
fn common_config_false_bare_spellings_allow_absolute_worktree() {
let worktree = tempfile::tempdir().expect("worktree");
for bare in ["false", "no", "off", "", "0", "-0", "+0", "00", "0x0"] {
let common = write_common_config(
format!(
"[core]\n\tbare = {bare}\n\tworktree = {}\n",
git_config_path(worktree.path())
)
.as_bytes(),
);
assert_eq!(
inspect_plain_common_config_authority(common.path()).expect(bare),
CommonConfigAuthority::Worktree(worktree.path().to_path_buf()),
"authority result for core.bare={bare:?}"
);
}
}
#[test]
fn common_config_rejects_contradictory_or_malformed_bare_with_absolute_worktree() {
let worktree = tempfile::tempdir().expect("worktree");