Skip to content
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.

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

Fix: "TypeError: Cannot read property 'documentElement' of null" #239

Merged
merged 4 commits into from
Oct 10, 2017

Conversation

markstos
Copy link
Contributor

@markstos markstos commented Oct 9, 2017

See commit message in 8354683 and Issue #238 for details.

Before, these tests were spread out in 3 sub-sections. One was
not labeled and the other two were not labeled as being for
validatePostResponse.

Now, more tests for this function are grouped together and clearly
labeled, more consistent with how other tests in the file are handled.
…hole cert.

copy/pasting a large chunk of encrypted text throughout the test file
was error prone.

Instead, we use a much shorter variable name for for the chunk.
Before, given junk input, validatePostResponse would fail with:

   TypeError: Cannot read property 'documentElement' of null

Now we'll fail with:

    SAMLResponse is not valid base64-encoded XML

To make this work, we primarily just needed to as a simple additional error check,
but to throw the error properly, we needed to move a bit of logic into
a nearby promise, but keeping some variable definitions in the outer
scope where they continue to be expected.
@markstos markstos requested a review from pdspicer October 9, 2017 17:15
@markstos markstos merged commit 305afbd into master Oct 10, 2017
@markstos
Copy link
Contributor Author

I'm going to ahead and merge this while I'm working on this project, but peer-review is still welcome.

@cjbarth cjbarth deleted the issue-238-type-error branch March 15, 2021 18:29
@cjbarth cjbarth added the bug label May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants