Strip NUL characters from tagged#7291
Conversation
emilghittasv
force-pushed
the
fix-null-bytes-in-tagged
branch
2 times, most recently
from
f1a59d7 to
d479d87
Compare
emilghittasv
force-pushed
the
fix-null-bytes-in-tagged
branch
from
d479d87 to
7562598
Compare
|
|
||
| if not ( | ||
| ((request.method == "GET") and (watch_secret := request.GET.get("watch", None))) | ||
| ((request.method == "GET") and (watch_secret := strip_nul_bytes(request.GET.get("watch", None)))) |
There was a problem hiding this comment.
None is not needed in the get() method. It's the default value
| @cache_page(60 * 60 * 24) # One day. | ||
| def api(request): | ||
| s = request.GET.get("s", None) | ||
| s = strip_nul_bytes(request.GET.get("s", None)) |
There was a problem hiding this comment.
None is not needed in the get() method. It's the default value
| """An API to provide auto-complete data for user names.""" | ||
| term = request.GET.get("term", "") | ||
| query = request.GET.get("query", "") | ||
| term = strip_nul_bytes(request.GET.get("term", "")) |
There was a problem hiding this comment.
Some calls like this one, pass an empty string as a default value and others rely on None. We should be consistent
There was a problem hiding this comment.
A good way to enforce this is by adding a str -> str contract in the signature of the function
| pass | ||
|
|
||
|
|
||
| def strip_nul_bytes(value): |
There was a problem hiding this comment.
My main concern with this PR is that we need to remember to sanitize which leaves a lot of room for errors. For example this PR already missed a call I believe in wiki/views. I suspect this will happen often. We could use a middleware for GET and POST requests only.
|
@escattone can you take a look on this ? |
2 participants
No description provided.