Skip to content

Conversation

@vagisha
Copy link
Collaborator

@vagisha vagisha commented Mar 14, 2025

Rationale

Properly escape folder names passed as parameter to JS function, otherwise names containing single quotes cause JavaScript error.

@vagisha vagisha requested a review from labkey-jeckels March 14, 2025 17:16
pageConfig.addHandler(spanId, "click", "viewExperimentDetails(this,'" + container.getPath() + "', '" + id + "','" + detailsPage + "')");
pageConfig.addHandler(spanId, "click", "viewExperimentDetails(this,"
+ PageFlowUtil.jsString(container.getPath())
+ ", '" + id + "', "
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well handle them all the same way.

Suggested change
+ ", '" + id + "', "
+ ", " + PageFlowUtil.jsString(id) + ", "

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

'id' is a non-null Integer so does not need to be quoted, I think.

I added some HTML-escaping in related code. If you could take another look that would be great.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I somehow thought it was a string.

@vagisha vagisha requested a review from labkey-jeckels March 20, 2025 19:44
Co-authored-by: Josh Eckels <jeckels@labkey.com>
@vagisha vagisha merged commit 27764f0 into release24.11-SNAPSHOT Mar 22, 2025
1 check passed
@vagisha vagisha deleted the 24.11_fb_js-escape-viewExperimentDetails branch March 22, 2025 05: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.

3 participants