rest: Disable caching for all REST API endpoints.

Investigation into #12876, a mysterious bug where users were seeing
messages reappear as unread, determined that the root cause was
missing headers to disable client-side caching for Zulip's REST API
endpoints.

This manifested, in particular, for `GET /messages`, which is
essentially the only API GET endpoint used by the webapp at all.  When
using the `Ctrl+Shift+T` feature of browsers to restore a recently
closed tab (and potentially other code paths), the browser would
return from its disk cache a cached copy of the GET /messages results.

Because we include message flags on messages fetched from the server,
this in particular meant that those tabs would get a stale version of
the unread flag for the batches of the most recent ~1200 messages that
Zulip fetches upon opening a new browser tab.

The issue took same care to reproduce as well, in large part because
the arguments to those initial GET /messages requests will vary as one
reads messages (because the `pointer` moves forward) and then enters
the "All messages" view; the disk cache is only used for GET requests
with the exact same URL parameters.

We will probably still want to merge the events error-handling changes
we had previously proposed for this, but the conclusion of this being
a straightforward case of missing cache-control headers is much more
satisfying than the "badly behaving Chrome" theory discussed in the
issue thread.

Fixes #12876.
This commit is contained in:
Tim Abbott 2019-07-25 12:03:35 -07:00
parent 6d5a20ac62
commit b8a1050fc4
1 changed files with 25 additions and 2 deletions

View File

@ -1,12 +1,15 @@
from typing import Any, Dict
from functools import wraps
from typing import Any, Callable, Dict
from django.utils.module_loading import import_string
from django.utils.translation import ugettext as _
from django.utils.cache import add_never_cache_headers
from django.views.decorators.csrf import csrf_exempt, csrf_protect
from zerver.decorator import authenticated_json_view, authenticated_rest_api_view, \
process_as_post, authenticated_uploads_api_view
process_as_post, authenticated_uploads_api_view, RespondAsynchronously, \
ReturnT
from zerver.lib.response import json_method_not_allowed, json_unauthorized
from django.http import HttpRequest, HttpResponse, HttpResponseRedirect
from django.conf import settings
@ -14,6 +17,26 @@ from django.conf import settings
METHODS = ('GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'PATCH')
FLAGS = ('override_api_url_scheme')
def never_cache_responses(view_func: Callable[..., ReturnT]) -> Callable[..., ReturnT]:
"""Patched version of the standard Django decorator that adds headers
to a response so that it will never be cached.
We need to patch this because our Django+Tornado
RespondAsynchronously hack involves returning a value that isn't a
Django response object, on which add_never_cache_headers would
crash. This only occurs in a case where client-side caching
wouldn't be possible anyway (we aren't returning a response to the
client yet -- it's for longpolling).
"""
@wraps(view_func)
def _wrapped_view_func(request: HttpRequest, *args: Any, **kwargs: Any) -> ReturnT:
response = view_func(request, *args, **kwargs)
if response is not RespondAsynchronously:
add_never_cache_headers(response)
return response
return _wrapped_view_func
@never_cache_responses
@csrf_exempt
def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
"""Dispatch to a REST API endpoint.