aleleba/soundcraft
1
0
Fork 0
mirror of https://github.com/storytold/soundcraft.git synced 2026-10-30 05:26:52 -06:00
Code Issues Packages Projects Releases Wiki Activity

score: apply every byte-budget and per-part cap in the same place

Browse Source 
Addresses the review threads on #993:

- MAX_NOTES_PER_PART: the layout loop stops collecting past 100k notes
  per part, so the raw note vector cannot grow unboundedly regardless
  of clip count.
- fill: a pitch the chord cap rejects no longer extends the onset end
  (e.0), so dropped pitches do not inflate chord duration; a retained
  pitch that reappears with a longer duration still does.
- musicxml now checks the budget before the per-measure loop (covers an
  oversized title/composer, escaped, even with zero measures), after the
  part-list, and at the end, so no growth path is unchecked.
- svg now checks every measure (rest-only or missing measures cannot
  grow the output untouched past the budget), every staff, every system
  and the final footer, so staff/clef/bracket growth is capped too.
- over_budget takes the budget as a parameter and the budgeted entry
  points stay separate so tests pass a tiny budget instead of building
  256 MiB. The oversize test shrank from ~256 MiB to ~64 KiB and the
  new header test checks the title/composer escape path directly.

Regression tests.
This commit is contained in:
Samuel Bacaro
2026-10-25 09:23:40 -03:00
parent ccd3fe7ca2
commit fa77e03e16
1 changed files with 72 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/score.rs
+72 -21
View File
@@ -19,13 +19,16 @@ const MAX_OUTPUT_BYTES: usize = 256 * 1024 * 1024; // 256 MiB
const MAX_ITEMS_PER_MEASURE: usize = 128;
/// Upper bound on pitches per chord, so a hostile session cannot make a huge document.
const MAX_PITCHES_PER_CHORD: usize = 16;
/// Upper bound on notes collected per part, before `fill` builds onsets and spans, so a hostile
/// session cannot make `layout` accumulate an unbounded note vector.
const MAX_NOTES_PER_PART: usize = 100_000;
/// Note values a single notehead can show, in sixteenths (longest first).
const VALUES: [u32; 8] = [16, 12, 8, 6, 4, 3, 2, 1];
/// The output has grown past the byte budget: refuse to grow it further.
fn over_budget(buf: &str) -> Result<(), String> {
if buf.len() > MAX_OUTPUT_BYTES {
return Err(format!("score too large: over {} MiB", MAX_OUTPUT_BYTES / (1024 * 1024)));
fn over_budget(buf: &str, budget: usize) -> Result<(), String> {
if buf.len() > budget {
return Err(format!("score too large: over {} bytes", budget));
}
Ok(())
}
@@ -107,6 +110,9 @@ pub fn layout(s: &Session, tracks: &[TrackId]) -> Vec<Part> {
let base = s.tempo.samples_to_ticks(c.start, sr);
let end = s.tempo.samples_to_ticks(c.start.saturating_add(c.length), sr);
for n in &sequence.notes {
if notes.len() >= MAX_NOTES_PER_PART {
break;
}
let st = base.saturating_add(n.start);
if st < base || st >= end || n.length <= 0 {
continue;
@@ -165,17 +171,21 @@ fn split_units(mut units: i64) -> Vec<u32> {
}
fn fill(bars: &[(i64, i64, u32, u32)], notes: &[(i64, i64, u8)]) -> Vec<Measure> {
// Onsets → chords, each held until its longest note ends or the next onset.
// Onsets → chords, each held until its longest *retained* note ends or the next onset. A pitch
// the cap rejects must not extend the onset end.
let mut onsets: std::collections::BTreeMap<i64, (i64, Vec<u8>)> = std::collections::BTreeMap::new();
for (a, b, p) in notes {
let e = onsets.entry(*a).or_insert((*b, Vec::new()));
e.0 = e.0.max(*b);
if !e.1.contains(p) {
e.1.push(*p);
let (ends, pitches) = onsets.entry(*a).or_insert((*b, Vec::new()));
let new_pitch = !pitches.contains(p);
if new_pitch && pitches.len() >= MAX_PITCHES_PER_CHORD {
// Chord full and this pitch is absent: it is dropped and must not extend the onset end.
continue;
}
// Cap chord size to avoid pathological memory use.
if e.1.len() > MAX_PITCHES_PER_CHORD {
e.1.truncate(MAX_PITCHES_PER_CHORD);
// A retained pitch (already in the chord, or one the cap accepts) may extend the onset
// end, even when it reappears with a longer duration.
*ends = (*ends).max(*b);
if new_pitch {
pitches.push(*p);
}
}
let starts: Vec<i64> = onsets.keys().copied().collect();
@@ -258,10 +268,19 @@ fn note_type(units: u32) -> (&'static str, bool) {
/// MusicXML 4.0 (partwise) for `parts`.
pub fn musicxml(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
musicxml_budgeted(parts, setup, MAX_OUTPUT_BYTES)
}
/// [`musicxml`] with an explicit byte budget (kept separate so tests can pass a tiny budget
/// instead of building a 256 MiB score).
fn musicxml_budgeted(parts: &[Part], setup: &ScoreSetup, budget: usize) -> Result<String, String> {
let mut x = String::new();
x.push_str("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n");
x.push_str("<!DOCTYPE score-partwise PUBLIC \"-//Recordare//DTD MusicXML 4.0 Partwise//EN\" \"http://www.musicxml.org/dtds/partwise.dtd\">\n");
x.push_str("<score-partwise version=\"4.0\">\n");
// The header (and an oversized title/composer, escaped) can itself exceed the budget even with
// no measures, so check before the per-measure loop too.
over_budget(&x, budget)?;
let _ = writeln!(x, " <work><work-title>{}</work-title></work>", xml_escape(&setup.title));
x.push_str(" <identification>\n");
if !setup.composer.is_empty() {
@@ -272,6 +291,7 @@ pub fn musicxml(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
let _ = writeln!(x, " <score-part id=\"P{}\"><part-name>{}</part-name></score-part>", i + 1, xml_escape(&p.name));
}
x.push_str(" </part-list>\n");
over_budget(&x, budget)?;
for (i, p) in parts.iter().enumerate() {
let _ = writeln!(x, " <part id=\"P{}\">", i + 1);
let mut meter = (0, 0);
@@ -338,11 +358,12 @@ pub fn musicxml(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
}
}
x.push_str(" </measure>\n");
over_budget(&x)?;
over_budget(&x, budget)?;
}
x.push_str(" </part>\n");
}
x.push_str("</score-partwise>\n");
over_budget(&x, budget)?;
Ok(x)
}
@@ -356,6 +377,12 @@ fn staff_step(pitch: u8, bass: bool) -> i32 {
/// An engraved score as a printable SVG page (A4 portrait width, as tall as needed).
pub fn svg(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
svg_budgeted(parts, setup, MAX_OUTPUT_BYTES)
}
/// [`svg`] with an explicit byte budget (kept separate so tests can pass a tiny budget instead of
/// building a 256 MiB score).
fn svg_budgeted(parts: &[Part], setup: &ScoreSetup, budget: usize) -> Result<String, String> {
const W: f64 = 794.0;
const MARGIN: f64 = 48.0;
const SP: f64 = 7.0; // staff space
@@ -377,7 +404,9 @@ pub fn svg(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
let _ = writeln!(o, "<text x=\"{}\" y=\"56\" font-size=\"24\" text-anchor=\"middle\">{}</text>", W / 2.0, xml_escape(&setup.title));
if !setup.composer.is_empty() {
let _ = writeln!(o, "<text x=\"{}\" y=\"84\" font-size=\"13\" text-anchor=\"end\">{}</text>", W - MARGIN, xml_escape(&setup.composer));
over_budget(&o, budget)?;
}
over_budget(&o, budget)?;
let bar_w = (staff_x1 - staff_x0) / per as f64;
for sys in 0..systems {
let y_sys = top + sys as f64 * sys_h;
@@ -489,8 +518,13 @@ pub fn svg(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
let _ = writeln!(o, "<path d=\"M{:.1} {y:.1} q 8 5 16 0\" fill=\"none\" stroke=\"black\" stroke-width=\"0.9\"/>", x + 3.0);
}
}
over_budget(&o)?;
// Check each measure, so a run of rest-only or missing measures that only draws
// bar lines cannot grow the output past the budget untouched.
over_budget(&o, budget)?;
}
// The staff markup (lines, clef, track names) itself grows with the part count even
// without any items, so check once per staff too.
over_budget(&o, budget)?;
}
// System bracket.
let y_last = y_sys + (parts.len().max(1) - 1) as f64 * STAFF_GAP + 4.0 * SP;
@@ -500,8 +534,10 @@ pub fn svg(parts: &[Part], setup: &ScoreSetup) -> Result<String, String> {
staff_x0 - 30.0,
staff_x0 - 30.0
);
over_budget(&o, budget)?;
}
o.push_str("</svg>\n");
over_budget(&o, budget)?;
Ok(o)
}
@@ -606,22 +642,37 @@ mod tests {
#[test]
fn oversize_score_is_refused_not_aborted() {
// Many dense parts drive the output past the byte budget: the call must
// return an error, not grow a multi-GB string or abort.
let bars: Vec<_> = (0..MAX_MEASURES as i64).map(|i| (i * 16, 16, 4, 4)).collect();
// Many dense parts drive the output past the byte budget: the call must return an error,
// not grow a multi-GB string or abort. Use the budgeted entry points with a tiny budget so
// the test exercises the over-budget path without allocating 256 MiB of score, and covers
// both the per-measure checks and the header/staff-only growth.
let bars: Vec<_> = (0..100).map(|i| (i * 16, 16, 4, 4)).collect();
let mut notes = Vec::new();
for i in 0..(MAX_MEASURES * 8) as i64 {
for i in 0..2000 {
let onset = i * 2;
notes.push((onset, onset + 1, 60 + (i % 24) as u8));
}
let measures = fill(&bars, &notes);
let part = Part { name: "dense".into(), bass: false, measures };
let setup = ScoreSetup { title: "t".into(), composer: String::new(), bars_per_system: 1, show_track_names: true };
// Enough copies of the dense part to exceed the byte budget.
let parts: Vec<Part> = (0..256).map(|i| Part { name: format!("part{i}"), ..part.clone() }).collect();
let err = svg(&parts, &setup).unwrap_err();
// Enough copies of the dense part to exceed a tiny budget.
let parts: Vec<Part> = (0..64).map(|i| Part { name: format!("part{i}"), ..part.clone() }).collect();
let tiny = 1024;
let err = svg_budgeted(&parts, &setup, tiny).unwrap_err();
assert!(err.contains("score too large"), "{err}");
let err = musicxml(&parts, &setup).unwrap_err();
let err = musicxml_budgeted(&parts, &setup, tiny).unwrap_err();
assert!(err.contains("score too large"), "{err}");
}
#[test]
fn oversized_title_is_refused_by_the_header_check() {
// An oversized part name/title must be caught by the header check even with no measures:
// the escaped copy alone exceeds the budget. (A 54 MiB title blows past 64 KiB of escaped
// text without touching the per-measure loop.)
let setup = ScoreSetup { title: "&".repeat(64 * 1024), composer: "c".repeat(64 * 1024), bars_per_system: 4, show_track_names: false };
let err = musicxml_budgeted(&[], &setup, 1024).unwrap_err();
assert!(err.contains("score too large"), "{err}");
let err = svg_budgeted(&[], &setup, 1024).unwrap_err();
assert!(err.contains("score too large"), "{err}");
}
}