From c611aa1218437dd98cb1145a73fb420041be6b85 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Sat, 19 Sep 2026 09:43:25 -0500 Subject: [PATCH 1/3] test: cover the toPem() cases node-saml's suite has and ours did not node-saml hands its PEM parsing to toPem() in node-saml/node-saml#409 and will drop its copies of these cases once they ship here. Every case already behaves as asserted; no code changes. Closes #607 Co-Authored-By: Claude Opus 5 --- test/utils-tests.spec.ts | 75 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/utils-tests.spec.ts b/test/utils-tests.spec.ts index 8da5baf6..fcd8218c 100644 --- a/test/utils-tests.spec.ts +++ b/test/utils-tests.spec.ts @@ -86,6 +86,8 @@ describe("Utils tests", function () { "a line break between quanta": "QUJD\nREVG", "a line break anywhere in the data": "QU\nJDRE\nVG", "blanks in the data": " QU JD\tREVG ", + // RFC 7468 Figure 1 'base64finl': https://www.rfc-editor.org/rfc/rfc7468#section-3 + "a pad split across a line ending": "QUJDCg=\n=", }; const rejected = { @@ -101,6 +103,8 @@ describe("Utils tests", function () { "a character outside the base64 alphabet": "QU-JD", "nothing at all": "", "only blanks": " ", + "only padding": "==", + "a padded line with more data after it": "QUJDCg==\nQUJD", }; Object.entries(accepted).forEach(([description, data]) => { @@ -200,6 +204,32 @@ describe("Utils tests", function () { ); }); + for (const [name, prefix] of [ + ["a UTF-8 BOM", Buffer.from([0xef, 0xbb, 0xbf])], + ["blank lines", Buffer.from("\r\n \n")], + ] as const) { + it(`a Buffer holding a PEM file that opens with ${name}, rather than DER`, function () { + const file = Buffer.concat([prefix, fs.readFileSync("./test/static/client_public.pem")]); + + expect(utils.toPem(file)).to.equal(normalizedPem); + }); + } + + for (const [name, value, label] of [ + ["blank lines around a message", `\n\n${normalizedPem}\n\n`, undefined], + // node-saml/node-saml#361 + ["a line ending after bare base64", `${body.join("\n")}\n`, "CERTIFICATE"], + [ + "blanks and CRLF around bare base64", + `\r\n \r\n${body.join("\r\n")}\r\n\t\r\n`, + "CERTIFICATE", + ], + ] as const) { + it(name, function () { + expect(utils.toPem(value, label)).to.equal(normalizedPem); + }); + } + it("several certificates in one value", function () { const bundle = fs.readFileSync("./test/static/client_bundle.pem", "latin1"); @@ -382,6 +412,51 @@ describe("Utils tests", function () { ); }); }); + + describe("rejects a value it cannot account for whole", function () { + const normalizedPem = fs.readFileSync("./test/static/client_public.pem", "latin1"); + const lines = normalizedPem.trim().split("\n"); + const header = lines[0]; + const footer = lines[lines.length - 1]; + const body = lines.slice(1, -1); + const last = body.length - 1; + + for (const [place, value] of [ + ["the header's line", [`${header}${body[0]}`, ...body.slice(1), footer].join("\n")], + [ + "the footer's line", + [header, ...body.slice(0, last), `${body[last]}${footer}`].join("\n"), + ], + ["both boundaries' line", `${header}${body.join("")}${footer}`], + ] as const) { + it(`data on ${place}`, function () { + expect(() => utils.toPem(value)).to.throw("Invalid PEM format."); + }); + } + + it("a message that is opened and never closed", function () { + // A ReDoS regression in node-saml. What is asserted is the rejection, since a timer would + // measure the runner rather than the pattern. + const unclosed = `-----BEGIN CERTIFICATE-----\r\n${"AAAA\r\n".repeat(26)}!`; + + expect(() => utils.toPem(unclosed)).to.throw("Invalid PEM format."); + }); + + it("a header followed directly by its footer", function () { + expect(() => utils.toPem(`${header}\n${footer}\n`)).to.throw("Invalid PEM format."); + }); + + for (const [place, value] of [ + ["before the message", `subject=/CN=client\n${normalizedPem}`], + ["after the message", `${normalizedPem}Issued for testing.\n`], + ["between two messages", `${normalizedPem}and its issuer:\n${normalizedPem}`], + ] as const) { + it(`explanatory text ${place}, which pemCertificates() passes over`, function () { + expect(() => utils.toPem(value)).to.throw("Invalid PEM format."); + expect(utils.pemCertificates(value)).to.not.be.empty; + }); + } + }); }); describe("pemToDer", function () { From d6372d0cf0d784d3b1d168473d1e69b2330831ec Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Sat, 19 Sep 2026 10:32:37 -0500 Subject: [PATCH 2/3] test: drop an assertion and an issue link the scope review found redundant The pemCertificates() side of explanatory text is already tested, and a trailing line ending is not the tricky case an issue link is kept for. Co-Authored-By: Claude Opus 5 --- test/utils-tests.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/utils-tests.spec.ts b/test/utils-tests.spec.ts index fcd8218c..e5e5d7f7 100644 --- a/test/utils-tests.spec.ts +++ b/test/utils-tests.spec.ts @@ -217,7 +217,6 @@ describe("Utils tests", function () { for (const [name, value, label] of [ ["blank lines around a message", `\n\n${normalizedPem}\n\n`, undefined], - // node-saml/node-saml#361 ["a line ending after bare base64", `${body.join("\n")}\n`, "CERTIFICATE"], [ "blanks and CRLF around bare base64", @@ -451,9 +450,8 @@ describe("Utils tests", function () { ["after the message", `${normalizedPem}Issued for testing.\n`], ["between two messages", `${normalizedPem}and its issuer:\n${normalizedPem}`], ] as const) { - it(`explanatory text ${place}, which pemCertificates() passes over`, function () { + it(`explanatory text ${place}`, function () { expect(() => utils.toPem(value)).to.throw("Invalid PEM format."); - expect(utils.pemCertificates(value)).to.not.be.empty; }); } }); From fa96c99854af0ef67f160a0f6b6734fabecbb123 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Sat, 19 Sep 2026 11:33:36 -0500 Subject: [PATCH 3/3] test: shorten the ReDoS comment to one line Co-Authored-By: Claude Opus 5 --- test/utils-tests.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/utils-tests.spec.ts b/test/utils-tests.spec.ts index e5e5d7f7..f37305e5 100644 --- a/test/utils-tests.spec.ts +++ b/test/utils-tests.spec.ts @@ -434,8 +434,7 @@ describe("Utils tests", function () { } it("a message that is opened and never closed", function () { - // A ReDoS regression in node-saml. What is asserted is the rejection, since a timer would - // measure the runner rather than the pattern. + // node-saml's ReDoS regression; the rejection is asserted, not the time it takes. const unclosed = `-----BEGIN CERTIFICATE-----\r\n${"AAAA\r\n".repeat(26)}!`; expect(() => utils.toPem(unclosed)).to.throw("Invalid PEM format.");