Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor a2bdea55ce address review: register the manifest path before validation and keep the Chinese manifest fallback optional 2026-11-04 01:44:41 +00:00
Dylan M. Taylor 50852ceff1 ui-egui: embed craft-fonts files only from inside the checkout
The build hook canonicalised each manifest row's file and embedded it
with include_bytes! wherever it led, so an absolute path, a .. path or
a symlink in a craft-fonts checkout could put any readable file of the
build machine into the binary. Manifests and font files must now
resolve inside the canonical checkout directory.
2026-11-03 15:48:03 +00:00
6 changed files with 26 additions and 46 deletions
+1 -39
View File
@@ -8,13 +8,8 @@ use gridcraft_model::{Sheet, Workbook};
use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
/// Parsed number formats are cached by code.
pub(crate) fn parsed(code: &str) -> NumberFormat {
fn parsed(code: &str) -> NumberFormat {
static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
// A code of more bytes than 4 × the character limit can't be within it: it shows as General
// without being hashed or scanned again for every cell that shares it.
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse("General");
}
let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
if let Some(f) = lock().get(code) {
return f.clone();
@@ -29,12 +24,7 @@ pub(crate) fn parsed(code: &str) -> NumberFormat {
f
}
/// The parsed format of `code`; one longer than the character limit is a General format that keeps
/// the code it was made from. The engine itself formats through `parsed`, which doesn't copy it.
pub fn number_format(code: &str) -> NumberFormat {
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse(code);
}
parsed(code)
}
@@ -236,31 +226,3 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
}
s
}
#[cfg(test)]
mod tests {
use super::*;
/// A format code far past Excel's 255 characters shows its values as General, for every cell
/// that shares it, without being hashed into the cache each time.
#[test]
fn an_overlong_code_formats_as_general() {
let wb = Workbook::new();
let code = format!("0.00{}", "\"x\"".repeat(400_000));
for _ in 0..1_000 {
assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
}
assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
}
/// The public `number_format` keeps the code of an overlong format; it is General, but not the
/// same format as a real `General`.
#[test]
fn number_format_keeps_an_overlong_code() {
let code = format!("0.00{}", "\"x\"".repeat(2_000));
let f = number_format(&code);
assert!(f.is_general());
assert_eq!(f.code(), code);
assert_ne!(f, number_format("General"));
}
}
+1 -1
View File
@@ -443,7 +443,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result<Vec<u8>> {
let Some(sh) = wb.sheet(wb.active_sheet()) else { return Ok(Vec::new()) };
gridcraft_xlsx::write_csv(sh, wb.csv_delimiter.unwrap_or(b','), |c| match sh.value(c) {
gridcraft_core::Value::Number(n) if n.is_finite() => {
let shape = crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
let shape = crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
match shape.zip(gridcraft_core::date::datetime_from_serial(wb.date_system, n)) {
Some(((date, time), d)) => iso(&d, date, time),
None => n.to_string(),
+1 -1
View File
@@ -440,7 +440,7 @@ pub fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput {
Some(match v {
Value::Empty => return None,
Value::Text(t) if t.is_empty() => return None,
Value::Number(_) if crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
Value::Number(_) if crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
Value::Number(_) => ColType::Number,
Value::Text(_) => ColType::Text,
Value::Bool(_) => ColType::Bool,
+1 -1
View File
@@ -335,7 +335,7 @@ fn check_input(wb: &Workbook, input: &gridcraft_model::NamedInput, place: Place)
gridcraft_model::ColType::Number => v.is_number(),
gridcraft_model::ColType::Text => v.is_text(),
gridcraft_model::ColType::Bool => matches!(v, Value::Bool(_)),
gridcraft_model::ColType::Date => v.is_number() && crate::display::parsed(&date()).is_date(),
gridcraft_model::ColType::Date => v.is_number() && crate::display::number_format(&date()).is_date(),
};
if !ok {
wrong.push((c, holds(&v, ty)));
+1 -1
View File
@@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
for (n, a) in numeric.iter_mut().zip(any.iter()) {
*n = *n && *a;
}
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::parsed(f).is_date()).collect();
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::number_format(f).is_date()).collect();
Ok(Source { sheet: si, range: r, headers, formats, numeric, dates, rows })
}
+21 -3
View File
@@ -76,7 +76,21 @@ fn emit_craft_fonts() {
/// One `CraftFont { .. }` initialiser per manifest row GridCraft may embed.
fn craft_fonts(dir: &std::path::Path, wasm: bool, required: bool) -> Result<String, String> {
let manifest = dir.join("fonts/manifest.txt");
// Every file embedded comes from inside the checkout: a manifest row (or a symlink) naming a
// path outside it would put that file of the build machine into the binary.
let root = dir.canonicalize().map_err(|e| format!("{}: {e}", dir.display()))?;
let inside = |path: std::path::PathBuf| -> Result<std::path::PathBuf, String> {
let real = path.canonicalize().map_err(|e| format!("{}: {e}", path.display()))?;
if real.starts_with(&root) { Ok(real) } else { Err(format!("{} is outside the craft-fonts checkout {}", path.display(), root.display())) }
};
// Registered before validation, by the path as given (and its folder, to see the manifest appear
// or a symlink replaced): a build that skipped the fonts must rebuild once they are there.
let manifest_path = dir.join("fonts/manifest.txt");
println!("cargo::rerun-if-changed={}", manifest_path.display());
if let Some(parent) = manifest_path.parent() {
println!("cargo::rerun-if-changed={}", parent.display());
}
let manifest = inside(manifest_path)?;
println!("cargo::rerun-if-changed={}", manifest.display());
let text = std::fs::read_to_string(&manifest).map_err(|e| format!("{}: {e}", manifest.display()))?;
let rows = craft_fonts_policy::parse_manifest(&text)?;
@@ -84,7 +98,7 @@ fn craft_fonts(dir: &std::path::Path, wasm: bool, required: bool) -> Result<Stri
if required {
craft_fonts_policy::check_required(&rows)?;
}
let chinese = if wasm { Vec::new() } else { chinese_font_rows(dir, required)? };
let chinese = if wasm { Vec::new() } else { chinese_font_rows(&root, required)? };
for row in rows.iter().chain(&chinese) {
if craft_fonts_policy::barred(&row.family) {
println!("cargo::warning=craft-fonts: not embedding {} {} (repository asset policy)", row.family, row.style);
@@ -92,7 +106,7 @@ fn craft_fonts(dir: &std::path::Path, wasm: bool, required: bool) -> Result<Stri
}
let mut out = String::new();
for row in craft_fonts_policy::embeddable(&rows, wasm).into_iter().chain(craft_fonts_policy::embeddable(&chinese, wasm)) {
let path = dir.join(&row.file).canonicalize().map_err(|e| format!("{}: {e}", row.file))?;
let path = inside(root.join(&row.file)).map_err(|e| format!("{}: {e}", row.file))?;
println!("cargo::rerun-if-changed={}", path.display());
let scripts: Vec<String> = row.scripts.iter().map(|s| format!("{s:?}")).collect();
let family = &row.family;
@@ -120,6 +134,10 @@ fn chinese_font_rows(dir: &std::path::Path, required: bool) -> Result<Vec<craft_
Ok(Vec::new())
}
};
// `dir` is the canonical checkout: the manifest itself must not lead out of it either.
if manifest.canonicalize().is_ok_and(|real| !real.starts_with(dir)) {
return optional(format!("{} is outside the craft-fonts checkout", manifest.display()));
}
match std::fs::read_to_string(&manifest) {
Ok(text) => match craft_fonts_policy::parse_manifest(&text).and_then(|rows| {
craft_fonts_policy::check_declared_scripts(&rows)?;