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

sessionCheckEventListener should only call this.debug('got info from session check inframe', e) if origin is correct #616

Closed
Maximaximum opened this issue Aug 26, 2019 · 3 comments · Fixed by #617
Labels
bug For tagging faulty or unexpected behavior. pr-welcome We'd welcome a PR to solve the issue.

Comments

@Maximaximum
Copy link
Contributor

Describe the bug
Currently the setupSessionCheckEventListener() function logs ALL the events posted with window.postMessage():

this.debug('got info from session check inframe', e);

While this might make sense for events received from the session check frame, this makes absolutely no sense for messages received from other origins.

For example, in my application we use window.postMessage() for communication between iframes a lot. All the messages that are received from other sources (not from the session check frame) should be simply ignored by the oidc library.

This could have been not a huge issue, but since we're logging the whole event object (including all the source and target data), this leads to a huge memory leak. When the source iframe is detached from the DOM, it should be destroyed, but since a reference to its contentWindow is contained in the console now, the contentWindow cannot be garbage collected, so the whole iframe element is retained in memory.

@jeroenheijmans jeroenheijmans added bug For tagging faulty or unexpected behavior. pr-welcome We'd welcome a PR to solve the issue. labels Aug 27, 2019
@jeroenheijmans
Copy link
Collaborator

jeroenheijmans commented Aug 27, 2019

Makes sense. PR looks good to me. Thanks! 👍

@Maximaximum
Copy link
Contributor Author

@manfredsteyer @jeroenheijmans is this project still being maintained? Could you please review and merge outstanding PRs like this and release a new version? Do you need any help?

@jeroenheijmans
Copy link
Collaborator

I moderate issues and tend to post insights on PRs if I have any. At the moment I don't do merging, releasing, etc. AFAIK Manfred does so only whenever a new major Angular version comes out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For tagging faulty or unexpected behavior. pr-welcome We'd welcome a PR to solve the issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants