Reject unreopenable source before checked save stages files
This commit is contained in:
@@ -3,6 +3,9 @@ use serde::{Deserialize, Serialize};
|
|||||||
use std::{io::Write, path::Path};
|
use std::{io::Write, path::Path};
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
mod json_codec;
|
mod json_codec;
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "persistence/source_save_tests.rs"]
|
||||||
|
mod source_save_tests;
|
||||||
#[path = "migration_v2/mod.rs"]
|
#[path = "migration_v2/mod.rs"]
|
||||||
pub(crate) mod v2;
|
pub(crate) mod v2;
|
||||||
pub(crate) use v2::adapter::migrate as migrate_v2_limited;
|
pub(crate) use v2::adapter::migrate as migrate_v2_limited;
|
||||||
@@ -948,19 +951,54 @@ fn save_checked_profile(
|
|||||||
expected: Option<&[u8]>,
|
expected: Option<&[u8]>,
|
||||||
profile: json_codec::Profile,
|
profile: json_codec::Profile,
|
||||||
depth_128: bool,
|
depth_128: bool,
|
||||||
|
) -> Result<Vec<u8>, FileError> {
|
||||||
|
save_checked_profile_with_source(path, p, expected, profile, depth_128, |project| {
|
||||||
|
crate::language::format(project).into_bytes()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
// Keep the source writer replaceable only at this private seam so regressions
|
||||||
|
// can prove that malformed or lossy output never reaches filesystem staging.
|
||||||
|
fn save_checked_profile_with_source(
|
||||||
|
path: &Path,
|
||||||
|
p: &Project,
|
||||||
|
expected: Option<&[u8]>,
|
||||||
|
profile: json_codec::Profile,
|
||||||
|
depth_128: bool,
|
||||||
|
source: impl FnOnce(&Project) -> Vec<u8>,
|
||||||
) -> Result<Vec<u8>, FileError> {
|
) -> Result<Vec<u8>, FileError> {
|
||||||
validate_project(p)?;
|
validate_project(p)?;
|
||||||
let mut bytes = if path
|
let is_source = path
|
||||||
.extension()
|
.extension()
|
||||||
.is_some_and(|ext| ext.eq_ignore_ascii_case("arcadia"))
|
.is_some_and(|ext| ext.eq_ignore_ascii_case("arcadia"));
|
||||||
{
|
let mut bytes = if is_source {
|
||||||
crate::language::format(p).into_bytes()
|
source(p)
|
||||||
} else {
|
} else {
|
||||||
json_codec::encode(p, profile, depth_128)?
|
json_codec::encode(p, profile, depth_128)?
|
||||||
};
|
};
|
||||||
if !bytes.ends_with(b"\n") {
|
if !bytes.ends_with(b"\n") {
|
||||||
bytes.push(b'\n');
|
bytes.push(b'\n');
|
||||||
}
|
}
|
||||||
|
if is_source {
|
||||||
|
// Check exactly the final bytes, including their newline, through the
|
||||||
|
// public reader before creating a temp file or replacing a destination.
|
||||||
|
let decoded = decode_with_report_limited(
|
||||||
|
&bytes,
|
||||||
|
crate::language::ParseLimits {
|
||||||
|
max_source_bytes: profile.bytes,
|
||||||
|
..crate::language::ParseLimits::UNLIMITED
|
||||||
|
},
|
||||||
|
)?;
|
||||||
|
if decoded.project != *p {
|
||||||
|
return Err(FileError::Invalid(
|
||||||
|
"Generated source does not reproduce the complete requested project.".into(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
if decoded.migration.is_some() {
|
||||||
|
return Err(FileError::Invalid(
|
||||||
|
"Generated source unexpectedly requires migration.".into(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
let parent = path
|
let parent = path
|
||||||
.parent()
|
.parent()
|
||||||
.filter(|p| !p.as_os_str().is_empty())
|
.filter(|p| !p.as_os_str().is_empty())
|
||||||
|
|||||||
@@ -0,0 +1,145 @@
|
|||||||
|
//! Containment while the complete source8 adapter remains unavailable.
|
||||||
|
use super::*;
|
||||||
|
use crate::analysis::{Definition, DefinitionBody, QuantityAlgebra, QuantityKindDefinition};
|
||||||
|
use crate::analysis_numeric::Dimension;
|
||||||
|
use std::fs;
|
||||||
|
|
||||||
|
const ORDINARY_SOURCE7: &str = concat!(
|
||||||
|
"arcadia 7;\n",
|
||||||
|
"project saved \"Saved model\" id \"4210b9f2-7046-47f0-aa3b-153f626a579d\" {\n",
|
||||||
|
" description \"Retain café and exact identity\";\n",
|
||||||
|
"}",
|
||||||
|
);
|
||||||
|
|
||||||
|
fn ordinary() -> Project {
|
||||||
|
let decoded = decode_with_report(ORDINARY_SOURCE7.as_bytes()).unwrap();
|
||||||
|
assert!(decoded.migration.is_some());
|
||||||
|
decoded.project
|
||||||
|
}
|
||||||
|
|
||||||
|
fn directory_entries(path: &Path) -> Vec<std::ffi::OsString> {
|
||||||
|
let mut names = fs::read_dir(path)
|
||||||
|
.unwrap()
|
||||||
|
.map(|entry| entry.unwrap().file_name())
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
names.sort();
|
||||||
|
names
|
||||||
|
}
|
||||||
|
|
||||||
|
// A missing parent makes a mistakenly early NamedTempFile creation observable
|
||||||
|
// as Io, even when the temp would otherwise be removed again on error.
|
||||||
|
fn rejected_output(
|
||||||
|
project: &Project,
|
||||||
|
output: &[u8],
|
||||||
|
profile: json_codec::Profile,
|
||||||
|
check: impl Fn(&FileError),
|
||||||
|
) {
|
||||||
|
let root = tempfile::tempdir().unwrap();
|
||||||
|
let existing = root.path().join("existing.arcadia");
|
||||||
|
let sentinel = b"external bytes must survive\r\n";
|
||||||
|
fs::write(&existing, sentinel).unwrap();
|
||||||
|
for path in [
|
||||||
|
existing.clone(),
|
||||||
|
root.path().join("new.arcadia"),
|
||||||
|
root.path().join("missing-parent/new.arcadia"),
|
||||||
|
] {
|
||||||
|
let before = directory_entries(root.path());
|
||||||
|
let expected = (path == existing).then_some(sentinel.as_slice());
|
||||||
|
let error =
|
||||||
|
save_checked_profile_with_source(&path, project, expected, profile, false, |_| {
|
||||||
|
output.to_vec()
|
||||||
|
})
|
||||||
|
.unwrap_err();
|
||||||
|
check(&error);
|
||||||
|
assert_eq!(directory_entries(root.path()), before);
|
||||||
|
assert_eq!(fs::read(&existing).unwrap(), sentinel);
|
||||||
|
if path != existing {
|
||||||
|
assert!(!path.exists());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn malformed_generated_source_rejects_before_any_filesystem_staging() {
|
||||||
|
rejected_output(
|
||||||
|
&ordinary(),
|
||||||
|
b"arcadia",
|
||||||
|
json_codec::Profile::DOCUMENT,
|
||||||
|
|error| assert!(matches!(error, FileError::Text(_)), "{error}"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn parseable_source_omitting_a_definition_cannot_replace_the_whole_project() {
|
||||||
|
let mut project = ordinary();
|
||||||
|
let id = Id::parse_str("188b911c-5f8b-431c-9eea-2d41f3953767").unwrap();
|
||||||
|
project.definitions.insert(
|
||||||
|
id,
|
||||||
|
Definition {
|
||||||
|
id,
|
||||||
|
alias: "duration".into(),
|
||||||
|
name: "Duration".into(),
|
||||||
|
description: "Unused definitions are still authored model data.".into(),
|
||||||
|
body: DefinitionBody::QuantityKind(QuantityKindDefinition {
|
||||||
|
dimension: Dimension::new([0, 0, 1, 0, 0, 0, 0]).unwrap(),
|
||||||
|
algebra: QuantityAlgebra::Linear,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
validate_project(&project).unwrap();
|
||||||
|
let decoded = decode(ORDINARY_SOURCE7.as_bytes()).unwrap();
|
||||||
|
assert_eq!(decoded.id, project.id);
|
||||||
|
assert_eq!(decoded.name, project.name);
|
||||||
|
assert_eq!(decoded.elements, project.elements);
|
||||||
|
assert!(decoded.definitions.is_empty());
|
||||||
|
assert_ne!(decoded, project);
|
||||||
|
rejected_output(
|
||||||
|
&project,
|
||||||
|
ORDINARY_SOURCE7.as_bytes(),
|
||||||
|
json_codec::Profile::DOCUMENT,
|
||||||
|
|error| {
|
||||||
|
assert!(
|
||||||
|
matches!(error, FileError::Invalid(message)
|
||||||
|
if message == "Generated source does not reproduce the complete requested project."),
|
||||||
|
"{error}"
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn equal_project_from_generated_legacy_source_still_rejects_migration() {
|
||||||
|
rejected_output(
|
||||||
|
&ordinary(),
|
||||||
|
ORDINARY_SOURCE7.as_bytes(),
|
||||||
|
json_codec::Profile::DOCUMENT,
|
||||||
|
|error| {
|
||||||
|
assert!(
|
||||||
|
matches!(error, FileError::Invalid(message)
|
||||||
|
if message == "Generated source unexpectedly requires migration."),
|
||||||
|
"{error}"
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn generated_source_admission_checks_final_newline_under_the_effective_byte_limit() {
|
||||||
|
let project = ordinary();
|
||||||
|
let profile = json_codec::Profile::with_byte_limit(ORDINARY_SOURCE7.len());
|
||||||
|
rejected_output(&project, ORDINARY_SOURCE7.as_bytes(), profile, |error| {
|
||||||
|
assert!(matches!(error, FileError::Limit(_)), "{error}");
|
||||||
|
});
|
||||||
|
let mut terminated = ORDINARY_SOURCE7.as_bytes().to_vec();
|
||||||
|
terminated.push(b'\n');
|
||||||
|
let profile = json_codec::Profile::with_byte_limit(terminated.len());
|
||||||
|
// Existing final newline is not added twice: decoding now reaches the
|
||||||
|
// independent migration refusal, rather than exceeding this exact limit.
|
||||||
|
rejected_output(&project, &terminated, profile, |error| {
|
||||||
|
assert!(
|
||||||
|
matches!(error, FileError::Invalid(message)
|
||||||
|
if message == "Generated source unexpectedly requires migration."),
|
||||||
|
"{error}"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user