Skip to content

Add widget page support for placeholder-plain #246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sestr wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add widget page support for placeholder-plain #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/placeholder-plain/dist/getPageRef.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* @param {Buffer} pdfBuffer
* @param {Object} info As extracted from readRef()
* @param {Number} [pageNumber = 0] Desired page number
*/
export default function getPageRef(pdfBuffer: Buffer, info: any): string;
export default function getPageRef(pdfBuffer: Buffer, info: any, pageNumber?: number): any;
//# sourceMappingURL=getPageRef.d.ts.map
2 changes: 1 addition & 1 deletion packages/placeholder-plain/dist/getPageRef.d.ts.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions packages/placeholder-plain/dist/getPageRef.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Object.defineProperty(exports, "__esModule", {
value: true
});
exports.default = getPageRef;
var _utils = require("@signpdf/utils");
var _getPagesDictionaryRef = _interopRequireDefault(require("./getPagesDictionaryRef"));
var _findObject = _interopRequireDefault(require("./findObject"));
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
Expand All @@ -12,14 +13,19 @@ function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { de
*
* @param {Buffer} pdfBuffer
* @param {Object} info As extracted from readRef()
* @param {Number} [pageNumber = 0] Desired page number
*/
function getPageRef(pdfBuffer, info) {
function getPageRef(pdfBuffer, info, pageNumber = 0) {
const pagesRef = (0, _getPagesDictionaryRef.default)(info);
const pagesDictionary = (0, _findObject.default)(pdfBuffer, info.xref, pagesRef);
const kidsPosition = pagesDictionary.indexOf('/Kids');
const kidsStart = pagesDictionary.indexOf('[', kidsPosition) + 1;
const kidsEnd = pagesDictionary.indexOf(']', kidsPosition);
const pages = pagesDictionary.slice(kidsStart, kidsEnd).toString();
const split = pages.trim().split(' ', 3);
return `${split[0]} ${split[1]} ${split[2]}`;
const pagesSplit = [];
pages.trim().split(' ').forEach((v, i) => i % 3 === 0 ? pagesSplit.push([v]) : pagesSplit[pagesSplit.length - 1].push(v));
if (pageNumber < 0 || pagesSplit.length <= pageNumber) {
throw new _utils.SignPdfError(`Failed to get reference of page "${pageNumber}".`, _utils.SignPdfError.TYPE_INPUT);
}
return pagesSplit[pageNumber].join(' ');
}
6 changes: 5 additions & 1 deletion packages/placeholder-plain/dist/plainAddPlaceholder.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function plainAddPlaceholder({ pdfBuffer, reason, contactInfo, name, location, signingTime, signatureLength, subFilter, widgetRect, appName, }: InputType): Buffer;
export function plainAddPlaceholder({ pdfBuffer, reason, contactInfo, name, location, signingTime, signatureLength, subFilter, widgetRect, widgetPage, appName, }: InputType): Buffer;
export type InputType = {
pdfBuffer: Buffer;
reason: string;
Expand All @@ -15,6 +15,10 @@ export type InputType = {
* [x1, y1, x2, y2] widget rectangle
*/
widgetRect?: number[];
/**
* Page number where the widget should be placed
*/
widgetPage?: number;
/**
* Name of the application generating the signature
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/placeholder-plain/dist/plainAddPlaceholder.d.ts.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion packages/placeholder-plain/dist/plainAddPlaceholder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const getAcroFormRef = slice => {
* @property {number} [signatureLength]
* @property {string} [subFilter] One of SUBFILTER_* from \@signpdf/utils
* @property {number[]} [widgetRect] [x1, y1, x2, y2] widget rectangle
* @property {number} [widgetPage] Page number where the widget should be placed
* @property {string} [appName] Name of the application generating the signature
*/

Expand All @@ -64,11 +65,12 @@ const plainAddPlaceholder = ({
signatureLength = _utils.DEFAULT_SIGNATURE_LENGTH,
subFilter = _utils.SUBFILTER_ADOBE_PKCS7_DETACHED,
widgetRect = [0, 0, 0, 0],
widgetPage = 0,
appName = undefined
}) => {
let pdf = (0, _utils.removeTrailingNewLine)(pdfBuffer);
const info = (0, _readPdf.default)(pdf);
const pageRef = (0, _getPageRef.default)(pdf, info);
const pageRef = (0, _getPageRef.default)(pdf, info, widgetPage);
const pageIndex = (0, _getIndexFromRef.default)(info.xref, pageRef);
const addedReferences = new Map();
const pdfKitMock = {
Expand Down Expand Up @@ -109,6 +111,7 @@ const plainAddPlaceholder = ({
signatureLength,
subFilter,
widgetRect,
widgetPage,
appName
});
if (!getAcroFormRef(pdf.toString())) {
Expand Down
12 changes: 9 additions & 3 deletions packages/placeholder-plain/src/getPageRef.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {SignPdfError} from '@signpdf/utils';
import getPagesDictionaryRef from './getPagesDictionaryRef';
import findObject from './findObject';

Expand All @@ -6,14 +7,19 @@ import findObject from './findObject';
*
* @param {Buffer} pdfBuffer
* @param {Object} info As extracted from readRef()
* @param {Number} [pageNumber = 0] Desired page number
*/
export default function getPageRef(pdfBuffer, info) {
export default function getPageRef(pdfBuffer, info, pageNumber = 0) {
const pagesRef = getPagesDictionaryRef(info);
const pagesDictionary = findObject(pdfBuffer, info.xref, pagesRef);
const kidsPosition = pagesDictionary.indexOf('/Kids');
const kidsStart = pagesDictionary.indexOf('[', kidsPosition) + 1;
const kidsEnd = pagesDictionary.indexOf(']', kidsPosition);
const pages = pagesDictionary.slice(kidsStart, kidsEnd).toString();
const split = pages.trim().split(' ', 3);
return `${split[0]} ${split[1]} ${split[2]}`;
const pagesSplit = [];
pages.trim().split(' ').forEach((v, i) => (i % 3 === 0 ? pagesSplit.push([v]) : pagesSplit[pagesSplit.length - 1].push(v)));
Comment on lines +19 to +20
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /Kids parsing uses pages.trim().split(' '), which only splits on a single space and will break on valid PDFs that use other whitespace (newlines, tabs) or multiple spaces between tokens. This can mis-group the obj gen R triples and return the wrong page ref. Consider tokenizing with a whitespace regex (and filtering empty tokens) and then selecting the 3 tokens for pageNumber (or iterating until the desired index) with a clear length check.

Suggested change
const pagesSplit = [];
pages.trim().split(' ').forEach((v, i) => (i % 3 === 0 ? pagesSplit.push([v]) : pagesSplit[pagesSplit.length - 1].push(v)));
const tokens = pages.trim().split(/\s+/).filter(Boolean);
const pagesSplit = [];
for (let i = 0; i + 2 < tokens.length; i += 3) {
pagesSplit.push(tokens.slice(i, i + 3));
}

Copilot uses AI. Check for mistakes.
if (pageNumber < 0 || pagesSplit.length <= pageNumber) {
throw new SignPdfError(`Failed to get reference of page "${pageNumber}".`, SignPdfError.TYPE_INPUT);
}
return pagesSplit[pageNumber].join(' ');
Comment on lines +21 to +24
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pageNumber isn't validated to be a non-negative integer. If callers pass NaN, a float, or a string that coerces unexpectedly, the bounds check may not trigger and pagesSplit[pageNumber] can be undefined, causing a runtime TypeError on .join(...) instead of a SignPdfError. Validate/coerce upfront (e.g., Number.isInteger) and throw SignPdfError.TYPE_INPUT on invalid values.

Copilot uses AI. Check for mistakes.
}
5 changes: 4 additions & 1 deletion packages/placeholder-plain/src/plainAddPlaceholder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const getAcroFormRef = (slice) => {
* @property {number} [signatureLength]
* @property {string} [subFilter] One of SUBFILTER_* from \@signpdf/utils
* @property {number[]} [widgetRect] [x1, y1, x2, y2] widget rectangle
* @property {number} [widgetPage] Page number where the widget should be placed
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The widgetPage JSDoc says "Page number" but doesn’t specify whether it’s 0-based or 1-based. Since the implementation defaults to 0, clarify in the docstring (and/or error message) that this is a 0-based page index to avoid API confusion.

Suggested change
* @property {number} [widgetPage] Page number where the widget should be placed
* @property {number} [widgetPage] 0-based page index where the widget should be placed

Copilot uses AI. Check for mistakes.
* @property {string} [appName] Name of the application generating the signature
*/

Expand All @@ -67,11 +68,12 @@ export const plainAddPlaceholder = ({
signatureLength = DEFAULT_SIGNATURE_LENGTH,
subFilter = SUBFILTER_ADOBE_PKCS7_DETACHED,
widgetRect = [0, 0, 0, 0],
widgetPage = 0,
appName = undefined,
}) => {
let pdf = removeTrailingNewLine(pdfBuffer);
const info = readPdf(pdf);
const pageRef = getPageRef(pdf, info);
const pageRef = getPageRef(pdf, info, widgetPage);
const pageIndex = getIndexFromRef(info.xref, pageRef);
Comment on lines 70 to 77
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new widgetPage option changes behavior (allows placing the widget on non-first pages) but there’s no test covering multi-page PDFs or out-of-range page selection. Add/update tests to assert the annotation is added to the requested page (a multi-page fixture like resources/issue-158-test.pdf could work) and that invalid widgetPage values surface a SignPdfError.TYPE_INPUT.

Copilot uses AI. Check for mistakes.
const addedReferences = new Map();

Expand Down Expand Up @@ -126,6 +128,7 @@ export const plainAddPlaceholder = ({
signatureLength,
subFilter,
widgetRect,
widgetPage,
appName,
});

Expand Down
Loading