-
Notifications
You must be signed in to change notification settings - Fork 31
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
sanitize title and provider input string #1504
Conversation
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.
If we don't already, should we also sanitize inputs in the client?
(if yes, then let's do in a follow-up PR)
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.
@akihikokuroda is it possible to check for limit
and offset
values in jobs-list too?
https://github.com/Qiskit/qiskit-serverless/blob/main/gateway/api/views.py#L479
@Tansito I'm not sure if the PageNumberPagination handles by itself. Does DAST detect issue about it? If so, we can deal with it in a separate PR. |
@psschwei No, we don't. The query strings are almost passthrough to the gateway. |
gateway/api/utils.py
Outdated
if name: | ||
sanitized_name = "" | ||
for c in name: | ||
if c.isalnum() or c in ["_", "-", ":", "@", "/"]: |
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.
If this PR is only for title
and provider
we should only accept: ["_", "-"]
, WDYT?
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.
Maybe "/" in case we can receive: "provider/title" string, but I think we are not checking this here, right Aki?
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.
It is checking the "/" here. So we can accept "-", "_" and "/".
Let's try it! Thanks Aki 🎉 |
Summary
This PR adding the code that sanitizes the title and provider input string
Details and comments