Skip to content

Commit

Permalink
feat: wrap inflation errors
Browse files Browse the repository at this point in the history
`pako` has a tendency to throw obscure error strings e.g. "too many length or distance symbols", which can make gracefully handling bad input difficult when an error is thrown from `parseLoginRequest`.
This wraps the inflation step to allow throwing a dedicated error with a static message for inflation issues, while still making the original error available.
  • Loading branch information
mastermatt committed Feb 23, 2023
1 parent 49295a2 commit 67be597
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/flow.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { inflateString, base64Decode } from './utility';
import { inflateString, base64Decode, WrappedError } from './utility';
import { verifyTime } from './validator';
import libsaml from './libsaml';
import {
Expand Down Expand Up @@ -67,7 +67,13 @@ async function redirectFlow(options): Promise<FlowResult> {
return Promise.reject('ERR_REDIRECT_FLOW_BAD_ARGS');
}

const xmlString = inflateString(decodeURIComponent(content));
let xmlString: string

try {
xmlString = inflateString(decodeURIComponent(content));
} catch (cause) {
throw new WrappedError('ERR_FAILED_INFLATION', { cause })
}

// validate the xml
try {
Expand Down
15 changes: 15 additions & 0 deletions src/utility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@ import { inflate, deflate } from 'pako';

const BASE64_STR = 'base64';

/**
* An Error class with a "cause".
*
* This can be replaced by the native option, once the minimum supported version is >=16.9.0.
* https://nodejs.org/docs/latest-v16.x/api/errors.html#errorcause
*/
export class WrappedError extends Error {
public cause: any;

constructor(message: string, options: { cause: any }) {
super(message);
this.cause = options.cause;
}
}

/**
* @desc Mimic lodash.zipObject
* @param arr1 {string[]}
Expand Down
8 changes: 8 additions & 0 deletions test/flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1382,3 +1382,11 @@ test.serial('should not throw ERR_SUBJECT_UNCONFIRMED for the expired SAML respo
}

});

test('should throw ERR_FAILED_INFLATION when pako is unable to inflate a redirect login request', async t => {
const query = { SAMLRequest: 'foo', Signature: 'bar', SigAlg: 'baz' };
const error = await t.throwsAsync<any>(idp.parseLoginRequest(sp, 'redirect', { query }));

t.is(error.message, 'ERR_FAILED_INFLATION');
t.is(error.cause, 'invalid block type'); // pako throws strings instead of Error instances for bad input
});

0 comments on commit 67be597

Please sign in to comment.