We've already got a bunch of other comments on work we need to do for
this decorator and an open issue that will ensure we at some point
rework this and add tests for it. In the meantime, I'd like to lock
down the rest of decorator.py at 100% coverage.
Fixes#1000.
This exception class was clearly missing the part where `role` gets
stored, which was intended to be inherited from
InvalidZulipServerError.
This fixes an unnecessary 500 error in the push notifications bouncer.
This completes the effort to ensure that all of our webhooks that do
parsing of the third-party message format log something that we can
use to debug cases where we're not parsing the payloads correctly.
In this commit:
Two new URLs are added, to make all realms accessible for server
admins. One is for the stats page itself and another for getting
chart data i.e. chart data API requests.
For the above two new URLs corresponding two view functions are
added.
This extends the /user_uploads API endpoint to support passing the
authentication credentials via the URL, not the HTTP_AUTHORIZATION
headers. This is an important workaround for the fact that React
Native's Webview system doesn't support setting HTTP_AUTHORIZATION;
the app will be responsible for rewriting URLs for uploaded files
directly to add this parameter.
This fixes a regression in 93678e89cd
and a4979410f9, where the webhooks using
authenticated_rest_api_view were migrated to a new model that didn't
include setting a custom Client string for the webhook.
When restoring these webhooks' client strings, we also fix places
where the client string was not capitalized the same was as the
product's name.
Many declarations were previously annotated with
Callable[..., HttpResponse]; this is equivalent to ViewFuncT, so here we
switch to it.
To enable this migration, the WrappedViewFuncT alias is removed; this is
equivalent to the simple & legible Callable[[ViewFuncT], ViewFuncT], so
for relatively no space change, a clearer return type is possible.
Originally was going to centralize this in zerver/lib/request.pyi, but this
file is not visible at run-time, being only a stub. The matching request.py
file seemed inappropriate, as it doesn't actually use ViewFuncT.
Webhook functions wrapped by the decorator:
@authenticated_api_view(is_webhook=True)
now log payloads that cause exceptions to webhook-errors.log.
Note that authenticated_api_view is only used by webhooks/github
and not anywhere else.
The name `create_logger` suggests something much bigger than what this
function actually does -- the logger doesn't any more or less exist
after the function is called than before. Its one real function is to
send logs to a specific file.
So, pull out that logic to an appropriately-named function just for
it. We already use `logging.getLogger` in a number of places to
simply get a logger by name, and the old `create_logger` callsites can
do the same.
Because calls to `create_logger` generally run after settings are
configured, these would override what we have in `settings.LOGGING` --
which in particular defeated any attempt to set log levels in
`test_settings.py`. Move all of these settings to the same place in
`settings.py`, so they can be overridden in a uniform way.
In python3 base64.b64decode() can take an ASCII string, and any
legit data will be ASCII. If you pass in non-ASCII data, the
function will properly throw a ValueError (verified in python3 shell).
>>> s = '안녕하세요'
>>> import base64
>>> base64.b64decode(s)
Traceback (most recent call last):
File "/srv/zulip-py3-venv/lib/python3.4/base64.py", line 37, in _bytes_from_decode_data
return s.encode('ascii')
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-4: ordinal not in range(128)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/srv/zulip-py3-venv/lib/python3.4/base64.py", line 83, in b64decode
s = _bytes_from_decode_data(s)
File "/srv/zulip-py3-venv/lib/python3.4/base64.py", line 39, in _bytes_from_decode_data
raise ValueError('string argument should contain only ASCII characters')
ValueError: string argument should contain only ASCII characters
This gets used when we call `process_client`, which we generally do at
some kind of login; and in particular, we do in the shared auth
codepath `login_or_register_remote_user`. Add a decorator to make it
easy, and use it on the various views that wind up there.
In particular, this ensures that the `query` is some reasonable
constant corresponding to the view, as intended. When not set, we
fall back in `update_user_activity` on the URL path, but in particular
for `log_into_subdomain` that can now contain a bunch of
request-specific data, which makes it (a) not aggregate properly, and
(b) not even fit in the `CHARACTER VARYING(50)` database field we've
allotted it.
The only place this attribute is used is in `update_user_activity`,
called only in `process_client`, which won't happen if we end up
returning a redirect just below. If we don't, we go and call
`add_logging_data` just after, which takes care of this already.
This won't work for all call paths without deeper refactoring,
but for at least some paths we can make this more direct -- function
arguments, rather than mutating a request attribute -- so it's easier
to see how the data is flowing.
FuncT was unused in decorator.py, and only imported into profile.py.
The @profiled decorator is now more strongly typed on return-type.
Annotations were converted to python3 format.
Now that every call site of check_subdomain produces its second
argument in exactly the same way, push that shared bit of logic
into a new wrapper for check_subdomain.
Also give that new function a name that says more specifically what
it's checking -- which I think is easier to articulate for this
interface than for that of check_subdomain.