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

Do not escape org name twice #6550

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Conversation

eaon
Copy link
Contributor

@eaon eaon commented Sep 19, 2022

Description of Changes

If InstanceConfig.organization_name is escaped before writing it to the database, we'd need to mark every use of it in the templates as | safe, which is more dubious than not escaping the database entry in the first place.

Fixes #6357

Testing

  • Log into JI with an admin user
  • Navigate to the Instance Configuration interface
  • Change the instance name to something that would be escaped (for example &s or ümläüts) and click Set Organization Name
  • When you get the Preferences saved. notification, none of the characters of the name you submitted are double-escaped
  • Navigate to the SI and verify the correct name is shown in the title

Checklist

  • I have written a test plan and validated it for this PR
  • These changes do not require documentation

@eaon eaon force-pushed the 6357-fix-double-escape branch from 553b89b to ed1196c Compare September 19, 2022 18:52
@eaon eaon marked this pull request as ready for review September 19, 2022 18:57
@eaon eaon requested a review from a team as a code owner September 19, 2022 18:57
@eaon eaon marked this pull request as draft September 19, 2022 19:12
If the entry is escaped, we'd need to mark every use of it in the
templates as `| safe` which is more dubious than not escaping the
database entry in the first place.

Fixes #6357
@eaon eaon force-pushed the 6357-fix-double-escape branch from ed1196c to 71a8b05 Compare September 19, 2022 19:43
@eaon eaon marked this pull request as ready for review September 19, 2022 20:05
@zenmonkeykstop zenmonkeykstop self-assigned this Sep 19, 2022
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

Test plan checks out, made a couple of attempts to try SQL injections which were shrugged off by SQLAlchemy without fuss!

@zenmonkeykstop zenmonkeykstop merged commit bee977b into develop Sep 21, 2022
@zenmonkeykstop zenmonkeykstop deleted the 6357-fix-double-escape branch September 21, 2022 15:18
@zenmonkeykstop zenmonkeykstop added this to the 2.5.0 milestone Sep 21, 2022
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.

Organization name gets escaped twice
2 participants