-
Notifications
You must be signed in to change notification settings - Fork 964
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
add a helper to project admin include for ultranormalization collisions #17124
base: main
Are you sure you want to change the base?
Conversation
return { | ||
"project": project, | ||
"project_name": project_name, | ||
"prohibited": prohibited, | ||
"collisions": [p.name for p in collisions], | ||
} |
Check warning
Code scanning / CodeQL
Reflected server-side cross-site scripting Medium
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 11 hours ago
To fix the problem, we need to escape the project_name
before including it in the returned dictionary. This can be done using the html.escape()
function from the html
module in Python, which is designed to escape special characters in strings to prevent XSS attacks.
We will import the html
module and use the html.escape()
function to sanitize the project_name
before it is included in the returned dictionary.
-
Copy modified line R50 -
Copy modified line R53
@@ -49,5 +49,6 @@ | ||
|
||
import html | ||
return { | ||
"project": project, | ||
"project_name": project_name, | ||
"project_name": html.escape(project_name), | ||
"prohibited": prohibited, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice stuff! I have sone suggestion for optimizing the query - it's not going to be materially impactful due to the low usage/latency of this view, but I'd be remiss if I didn't note it.
collisions = ( | ||
request.db.query(Project) | ||
.filter( | ||
func.ultranormalize_name(Project.name) | ||
== func.ultranormalize_name(project_name) | ||
) | ||
.filter( | ||
func.normalize_pep426_name(Project.name) | ||
!= func.normalize_pep426_name(project_name) | ||
) | ||
.all() | ||
) | ||
|
||
return {"project": project, "project_name": project_name, "prohibited": prohibited} | ||
return { | ||
"project": project, | ||
"project_name": project_name, | ||
"prohibited": prohibited, | ||
"collisions": [p.name for p in collisions], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimize: Since we're not using any part of the results other than the colliding name we can optimize and only return the name
part from the DB, saving data load/roundtrip, serialization work, etc.
This is more aligned with modern SQLAlchemy syntax, where we construct a statement and pass it to the session.
Needs an update to import select
as well
collisions = request.db.scalars(
select(Project.name)
.filter(
func.ultranormalize_name(Project.name)
== func.ultranormalize_name(project_name)
)
.filter(
func.normalize_pep426_name(Project.name)
!= func.normalize_pep426_name(project_name)
)
).all()
And then no need to listcomp "collisions"
later, since it'll be constructed as a list of strings.
resolves #17113
Example: