Skip to content

Conversation

@dballance
Copy link
Contributor

Description

Fixes a weird IE11 edge case, potentially related to IE11 handling of object references that traverse frames.

Motivation and Context

In IE11 we were unable to execute requests after authenticating. After authenticating and attempting to execute a request in IE, the below error was logged to the console and the "Loading" indicator stayed present in the UI. The HTTP request also never executed. I believe the error number indicates a NullPointerException in IE, but not sure.
image

I tracked the exception source down in swagger-js to:

export function applySecurities({ request, securities = {}, operation = {}, spec }) {
      ...
      const accessToken = token && token.access_token

At this point token was defined but token.access_token access threw an error. The debugger value was hidden (not able to view the properties of the object) and logging token displayed 'Permission Denied'.

This only occurred in IE, while chrome and others worked just fine. I'm still unsure the exact cause, but my hunch is that because the token object is originally generated in the oauth2 frame, there is some weird permission interaction with the parent frame. So, I attempted to break the reference and it seems to resolve the issue. This change would probably work in other places, specifically within the auth callbacks themselves, so we can move it if needed.

How Has This Been Tested?

Fixes the issue and IE and I see no regression in Chrome.

All existing tests pass in my fork:
image

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@shockey shockey self-requested a review January 18, 2018 00:30
@shockey
Copy link
Contributor

shockey commented Jan 18, 2018

Yeah, looks like IE has some window.opener object reference oddities: https://stackoverflow.com/a/30768443

@shockey shockey added this to the January 19, 2018 milestone Jan 18, 2018
@shockey shockey merged commit 286c0e0 into swagger-api:master Jan 19, 2018
@shockey
Copy link
Contributor

shockey commented Jan 19, 2018

thanks as always, @dballance!

@dballance dballance deleted the patch-1 branch January 19, 2018 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants