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.

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

Jump to bottom

Old iOS: getParentNode is not a function #516

Closed
pager-itc opened this issue Feb 16, 2021 · 7 comments
Closed

Old iOS: getParentNode is not a function #516

pager-itc opened this issue Feb 16, 2021 · 7 comments

Comments

@pager-itc
Copy link

pager-itc commented Feb 16, 2021 •
edited
Loading

On iOS 9 Safari DOMPurify.sanitize throws this error message (tested on iPhone 6S on BrowserStack, and physical iPad Mini 1 - iOS 9.3.5)

getParentNode is not a function. (In 'getParentNode(element)', 'getParentNode' is null)

On iOS 7 & 8 Safari (iPhone 5S & iPhone 6, BrowserStack)

null is not a function (evaluating 'getParentNode(element)')

Even in the most simple setup like below

<html>
<head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/dompurify/2.2.6/purify.js"></script>
  <script src="./index.js" defer></script>
</head>
<body>
  <div id="test"></div>
</body>
</html>
// index.js
const el = document.getElementById('test');
try {
  el.innerHTML = DOMPurify.sanitize('<div>Test</div>');
} catch (er) {
  el.innerText = er.message;
}

Transpiling with babel doesn't fix the issue.

The readme says that old browsers at the very least should do nothing. The isSupported property is true despite the error.
@cure53
Copy link
Owner

cure53 commented Feb 16, 2021

Ooof, you are right, while Safari 9 is not supported, we should not expose isSupported as true.

@securitum-mb do you have any idea how we can detect unsupported Safari?

@securityMB
Copy link

@cure53 My first thought is to check whether getParentNode is a function. If it doesn't, then the browser is not supported.

On the other hand (but unfortunately I don't have time to do it right now), maybe the clobbering protection as implemented was not the best idea. The implementation in Closure is slightly different and maybe a good place to start.

@cure53
Copy link
Owner

cure53 commented Feb 16, 2021

@pager-itc As far as I can see, this workaround should do.

  /**
   * Expose whether this browser supports running the full DOMPurify.
   */
  DOMPurify.isSupported =
    typeof getParentNode === 'function' &&
    implementation &&
    typeof implementation.createHTMLDocument !== 'undefined' &&
    documentMode !== 9;

It works as expected on our Safari 9 instance, canm you confirm that this does the trick for you and correctly sets isSupported?

@pager-itc
Copy link
Author

Can confirm; with that workaround there is no error, dompurify just returns the original HTML string as it was provided for the old Safari browsers and sanitizes as normal in Chrome and Firefox on my desktop.

cure53 added a commit that referenced this issue Feb 16, 2021
@cure53
Added better isSupported handling for Safari 9, see #516
a16346c
@cure53
Copy link
Owner

cure53 commented Feb 16, 2021

Cool, let me push that fix to main and see what the remaining test cases say.

@cure53 cure53 closed this as completed Feb 16, 2021
@ghost
Copy link

ghost commented Feb 17, 2021

Hey @cure53 @securityMB. Please check #514. It seems the issue of nullable getParentNode was fixed there. Please chekd the latest updates in the main branch if it works in your case

@cure53
Copy link
Owner

cure53 commented Feb 17, 2021

Aye, this one mostly addresses the faulty result for isSupported

@jlowcs jlowcs mentioned this issue Mar 12, 2021
Closed
This was referenced Mar 12, 2021
Closed
Merged
Merged
Merged
Closed
Closed
Closed
Merged
Closed
Closed
Closed
Merged
Closed
Merged
Merged
Merged
Closed
Closed
Closed
Open
This was referenced Mar 15, 2021
Merged
Merged
Closed
Merged
Merged
Closed
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@cure53 @securityMB @pager-itc