Skip to content

Commit 2945703

Browse files
committed
fix(pdf-server): one bad form field no longer aborts saving the rest
#577 dropped the per-field try/catch in buildAnnotatedPdfBytes when it swapped to type-dispatch, so the first field whose pdf-lib write throws (max-length text, missing /Yes appearance, radio buttonValue mapping to neither label nor index, ...) bubbled to the outer catch and silently dropped every subsequent field. Compounded by getAnnotatedPdfBytes passing every baseline field, so even an untouched problematic field in the PDF poisoned the whole save. - Re-wrap each field write in its own try/catch (warn + continue). - getAnnotatedPdfBytes now only sends fields whose value differs from baseline (still includes explicit ''/false sentinels for cleared fields so pdf-lib overwrites the original /V). - Regression test: a maxLength=2 text field fed a long string, followed by a normal field; assert the second one still lands.
1 parent 4fc9513 commit 2945703

3 files changed

Lines changed: 90 additions & 39 deletions

File tree

examples/pdf-server/src/mcp-app.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2971,15 +2971,23 @@ async function getAnnotatedPdfBytes(): Promise<Uint8Array> {
29712971
}
29722972
}
29732973

2974-
// buildAnnotatedPdfBytes gates on formFields.size > 0 and only writes
2975-
// entries present in the map. After clearAllItems() the map is empty →
2976-
// zero setText/uncheck calls → pdf-lib leaves original /V intact →
2977-
// the "stripped PDF" we promised keeps all its form data. To actually
2978-
// clear, send an explicit sentinel for every baseline field the user
2979-
// dropped: "" for text, false for checkbox (matching baseline type).
2980-
const formFieldsOut = new Map(formFieldValues);
2974+
// Only write fields that actually changed vs. what's already in the PDF.
2975+
// Unchanged fields are no-ops at best, and at worst trip pdf-lib edge
2976+
// cases (max-length text, missing /Yes appearance, …) on fields the user
2977+
// never touched — which, before the per-field catch in
2978+
// buildAnnotatedPdfBytes, aborted every subsequent field.
2979+
//
2980+
// Fields the user cleared (present in baseline, absent from formFieldValues
2981+
// after clearAllItems()) still need an explicit "" / false so pdf-lib
2982+
// overwrites the original /V instead of leaving it intact.
2983+
const formFieldsOut = new Map<string, string | boolean>();
2984+
for (const [name, value] of formFieldValues) {
2985+
if (pdfBaselineFormValues.get(name) !== value) {
2986+
formFieldsOut.set(name, value);
2987+
}
2988+
}
29812989
for (const [name, baselineValue] of pdfBaselineFormValues) {
2982-
if (!formFieldsOut.has(name)) {
2990+
if (!formFieldValues.has(name)) {
29832991
formFieldsOut.set(name, typeof baselineValue === "boolean" ? false : "");
29842992
}
29852993
}

examples/pdf-server/src/pdf-annotations.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,34 @@ describe("buildAnnotatedPdfBytes", () => {
11051105
const form = (await PDFDocument.load(out)).getForm();
11061106
expect(form.getTextField("name").getText()).toBe("kept");
11071107
});
1108+
1109+
it("a field that throws on write does not abort subsequent fields", async () => {
1110+
// Regression for #577: the per-field try/catch was dropped, so the
1111+
// first throwing field bubbled to the outer catch and silently dropped
1112+
// every field after it. setText() throws when value exceeds maxLength.
1113+
const doc = await PDFDocument.create();
1114+
const page = doc.addPage([612, 792]);
1115+
const form = doc.getForm();
1116+
const limited = form.createTextField("limited");
1117+
limited.setMaxLength(2);
1118+
limited.addToPage(page, { x: 10, y: 700 });
1119+
form.createTextField("after").addToPage(page, { x: 10, y: 660 });
1120+
const fixture = await doc.save();
1121+
1122+
const out = await buildAnnotatedPdfBytes(
1123+
fixture,
1124+
[],
1125+
new Map<string, string | boolean>([
1126+
["limited", "way too long"], // throws
1127+
["after", "kept"],
1128+
]),
1129+
);
1130+
1131+
const saved = (await PDFDocument.load(out)).getForm();
1132+
expect(saved.getTextField("after").getText()).toBe("kept");
1133+
// The throwing field is left at whatever pdf-lib could do with it —
1134+
// we only assert it didn't poison "after".
1135+
});
11081136
});
11091137
});
11101138

examples/pdf-server/src/pdf-annotations.ts

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
PDFCheckBox,
2323
PDFDropdown,
2424
PDFRadioGroup,
25+
type PDFForm,
2526
} from "pdf-lib";
2627

2728
// =============================================================================
@@ -815,44 +816,58 @@ export async function buildAnnotatedPdfBytes(
815816
await addAnnotationDicts(pdfDoc, annotations);
816817

817818
// Apply form fills. Dispatch on actual field type — getTextField(name) throws
818-
// for dropdowns/radios, so the old try/catch silently dropped those on save.
819+
// for dropdowns/radios, so we look up the generic field and instanceof it.
820+
// Each field is wrapped in its own try/catch: pdf-lib can throw on
821+
// length-constrained text, radios whose buttonValue maps to neither label
822+
// nor index, checkboxes missing a /Yes appearance, etc. One bad field must
823+
// not abort the rest of the loop (regressed in #577 when the inner catch
824+
// was dropped along with the type-specific getters).
819825
if (formFields.size > 0) {
826+
let form: PDFForm | undefined;
820827
try {
821-
const form = pdfDoc.getForm();
828+
form = pdfDoc.getForm();
829+
} catch {
830+
// No AcroForm in this PDF
831+
}
832+
if (form) {
822833
for (const [name, value] of formFields) {
823-
const field = form.getFieldMaybe(name);
824-
if (!field) continue;
825-
826-
if (field instanceof PDFCheckBox) {
827-
if (value) field.check();
828-
else field.uncheck();
829-
} else if (field instanceof PDFRadioGroup) {
830-
// The viewer stores pdf.js's buttonValue, which for PDFs with an
831-
// /Opt array is a numeric index ("0","1","2") rather than the
832-
// option label pdf-lib's select() expects. Try the label first,
833-
// then fall back to indexing into getOptions().
834-
const opts = field.getOptions();
835-
const s = String(value);
836-
if (opts.includes(s)) {
837-
field.select(s);
838-
} else {
839-
const idx = Number(s);
840-
if (Number.isInteger(idx) && idx >= 0 && idx < opts.length) {
841-
field.select(opts[idx]);
834+
try {
835+
const field = form.getFieldMaybe(name);
836+
if (!field) continue;
837+
838+
if (field instanceof PDFCheckBox) {
839+
if (value) field.check();
840+
else field.uncheck();
841+
} else if (field instanceof PDFRadioGroup) {
842+
// The viewer stores pdf.js's buttonValue, which for PDFs with an
843+
// /Opt array is a numeric index ("0","1","2") rather than the
844+
// option label pdf-lib's select() expects. Try the label first,
845+
// then fall back to indexing into getOptions().
846+
const opts = field.getOptions();
847+
const s = String(value);
848+
if (opts.includes(s)) {
849+
field.select(s);
850+
} else {
851+
const idx = Number(s);
852+
if (Number.isInteger(idx) && idx >= 0 && idx < opts.length) {
853+
field.select(opts[idx]);
854+
}
855+
// else: value is neither label nor index — leave unset
842856
}
843-
// else: value is neither label nor index — leave unset
857+
} else if (field instanceof PDFDropdown) {
858+
// select() auto-enables edit mode for values outside getOptions(),
859+
// so this works for both enumerated and free-text combos.
860+
field.select(String(value));
861+
} else if (field instanceof PDFTextField) {
862+
field.setText(String(value));
844863
}
845-
} else if (field instanceof PDFDropdown) {
846-
// select() auto-enables edit mode for values outside getOptions(),
847-
// so this works for both enumerated and free-text combos.
848-
field.select(String(value));
849-
} else if (field instanceof PDFTextField) {
850-
field.setText(String(value));
864+
// PDFButton, PDFOptionList, PDFSignature: no fill_form support yet
865+
} catch (err) {
866+
// Skip this field; carry on with the rest. Surfacing per-field
867+
// failures is the caller's job (see fill_form result), not save's.
868+
console.warn(`buildAnnotatedPdfBytes: skipped field "${name}":`, err);
851869
}
852-
// PDFButton, PDFOptionList, PDFSignature: no fill_form support yet
853870
}
854-
} catch {
855-
// pdfDoc.getForm() throws if the PDF has no AcroForm
856871
}
857872
}
858873

0 commit comments

Comments
 (0)