Skip to content

Conversation

@avivkeller
Copy link
Contributor

Description

Better XSS detection using URL.

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@avivkeller avivkeller requested a review from lsegal February 29, 2024 22:07
Comment on lines 8 to 15
try {
var match = decodeURIComponent(window.location.hash).match(/^#!(.+)/);
var name = match ? match[1] : '<%= url_for_main %>';
var url = new URL(name, location.href);
window.top.location.replace(url.origin === location.origin ? name : '<%= url_for_main %>');
} catch (e) {
window.top.location.replace('<%= url_for_main %>');
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try {
var match = decodeURIComponent(window.location.hash).match(/^#!(.+)/);
var name = match ? match[1] : '<%= url_for_main %>';
var url = new URL(name, location.href);
window.top.location.replace(url.origin === location.origin ? name : '<%= url_for_main %>');
} catch (e) {
window.top.location.replace('<%= url_for_main %>');
}
try {
var mainUrl = '<%= url_for_main %>';
var match = decodeURIComponent(window.location.hash).match(/^#!(.+)/);
var name = match ? match[1] : mainUrl;
var url = new URL(name, location.href);
window.top.location.replace(url.origin === location.origin ? name : mainUrl);
} catch (e) {
window.top.location.replace(mainUrl);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assignment must be outside the try catch loop

@lsegal
Copy link
Owner

lsegal commented Feb 29, 2024

LGTM

@avivkeller
Copy link
Contributor Author

Same here

@lsegal lsegal merged commit 1fcb2d8 into lsegal:main Feb 29, 2024
@avivkeller
Copy link
Contributor Author

Great! I'm sorry again for my initial mistake and I'm glad we worked it out!

@avivkeller avivkeller deleted the patch-2 branch February 29, 2024 22:25
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