Use If-Unmodified-Since istead of If-Match
If-Match relies on ETag, which depends on the Content-Encoding, which is more fragile given we updated the etag on save, while normal files are served by nginx. So this may occurs false mismatch.
This commit is contained in:
parent
894facc7e3
commit
6694306660
3 changed files with 26 additions and 36 deletions
|
@ -274,7 +274,7 @@ L.U.DataLayer = L.Evented.extend({
|
|||
if (!this.umap_id) return;
|
||||
this.map.get(this._dataUrl(), {
|
||||
callback: function (geojson, response) {
|
||||
this._etag = response.getResponseHeader('ETag');
|
||||
this._last_modified = response.getResponseHeader('Last-Modified');
|
||||
this.fromUmapGeoJSON(geojson);
|
||||
this.backupOptions();
|
||||
this.fire('loaded');
|
||||
|
@ -1017,7 +1017,7 @@ L.U.DataLayer = L.Evented.extend({
|
|||
data: formData,
|
||||
callback: function (data, response) {
|
||||
this._geojson = geojson;
|
||||
this._etag = response.getResponseHeader('ETag');
|
||||
this._last_modified = response.getResponseHeader('Last-Modified');
|
||||
this.setUmapId(data.id);
|
||||
this.updateOptions(data);
|
||||
this.backupOptions();
|
||||
|
@ -1028,7 +1028,7 @@ L.U.DataLayer = L.Evented.extend({
|
|||
this.map.continueSaving();
|
||||
},
|
||||
context: this,
|
||||
headers: {'If-Match': this._etag || ''}
|
||||
headers: this._last_modified ? {'If-Unmodified-Since': this._last_modified} : {}
|
||||
});
|
||||
},
|
||||
|
||||
|
|
|
@ -24,11 +24,8 @@ def post_data():
|
|||
def test_get(client, settings, datalayer):
|
||||
url = reverse('datalayer_view', args=(datalayer.pk, ))
|
||||
response = client.get(url)
|
||||
if getattr(settings, 'UMAP_XSENDFILE_HEADER', None):
|
||||
assert response['ETag'] is not None
|
||||
assert response['Last-Modified'] is not None
|
||||
assert response['Cache-Control'] is not None
|
||||
assert response['Vary'] == 'Accept-Encoding'
|
||||
assert 'Content-Encoding' not in response
|
||||
j = json.loads(response.content.decode())
|
||||
assert '_umap_options' in j
|
||||
|
@ -91,46 +88,44 @@ def test_should_not_be_possible_to_delete_with_wrong_map_id_in_url(client, datal
|
|||
def test_get_gzipped(client, datalayer, settings):
|
||||
url = reverse('datalayer_view', args=(datalayer.pk, ))
|
||||
response = client.get(url, HTTP_ACCEPT_ENCODING='gzip')
|
||||
if getattr(settings, 'UMAP_XSENDFILE_HEADER', None):
|
||||
assert response['ETag'] is not None
|
||||
assert response['Last-Modified'] is not None
|
||||
assert response['Cache-Control'] is not None
|
||||
assert response['Content-Encoding'] == 'gzip'
|
||||
|
||||
|
||||
def test_optimistic_concurrency_control_with_good_etag(client, datalayer, map, post_data): # noqa
|
||||
# Get Etag
|
||||
def test_optimistic_concurrency_control_with_good_last_modified(client, datalayer, map, post_data): # noqa
|
||||
# Get Last-Modified
|
||||
url = reverse('datalayer_view', args=(datalayer.pk, ))
|
||||
response = client.get(url)
|
||||
etag = response['ETag']
|
||||
last_modified = response['Last-Modified']
|
||||
url = reverse('datalayer_update',
|
||||
args=(map.pk, datalayer.pk))
|
||||
client.login(username=map.owner.username, password="123123")
|
||||
name = 'new name'
|
||||
post_data['name'] = 'new name'
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_MATCH=etag)
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_UNMODIFIED_SINCE=last_modified)
|
||||
assert response.status_code == 200
|
||||
modified_datalayer = DataLayer.objects.get(pk=datalayer.pk)
|
||||
assert modified_datalayer.name == name
|
||||
|
||||
|
||||
def test_optimistic_concurrency_control_with_bad_etag(client, datalayer, map, post_data): # noqa
|
||||
def test_optimistic_concurrency_control_with_bad_last_modified(client, datalayer, map, post_data): # noqa
|
||||
url = reverse('datalayer_update', args=(map.pk, datalayer.pk))
|
||||
client.login(username=map.owner.username, password='123123')
|
||||
name = 'new name'
|
||||
post_data['name'] = name
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_MATCH='xxx')
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_UNMODIFIED_SINCE='xxx')
|
||||
assert response.status_code == 412
|
||||
modified_datalayer = DataLayer.objects.get(pk=datalayer.pk)
|
||||
assert modified_datalayer.name != name
|
||||
|
||||
|
||||
def test_optimistic_concurrency_control_with_empty_etag(client, datalayer, map, post_data): # noqa
|
||||
def test_optimistic_concurrency_control_with_empty_last_modified(client, datalayer, map, post_data): # noqa
|
||||
url = reverse('datalayer_update', args=(map.pk, datalayer.pk))
|
||||
client.login(username=map.owner.username, password='123123')
|
||||
name = 'new name'
|
||||
post_data['name'] = name
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_MATCH=None)
|
||||
response = client.post(url, post_data, follow=True, HTTP_IF_UNMODIFIED_SINCE=None)
|
||||
assert response.status_code == 200
|
||||
modified_datalayer = DataLayer.objects.get(pk=datalayer.pk)
|
||||
assert modified_datalayer.name == name
|
||||
|
|
|
@ -693,12 +693,10 @@ class GZipMixin(object):
|
|||
def path(self):
|
||||
return self.object.geojson.path
|
||||
|
||||
def etag(self):
|
||||
# Align ETag with Nginx one, because when using X-Send-File, If-None-Match
|
||||
# and If-Modified-Since are handled by Nginx.
|
||||
# https://github.com/nginx/nginx/blob/4ace957c4e08bcbf9ef5e9f83b8e43458bead77f/src/http/ngx_http_core_module.c#L1675-L1709
|
||||
statobj = os.stat(self.path)
|
||||
return '"%x-%x"' % (int(statobj.st_mtime), statobj.st_size)
|
||||
@property
|
||||
def last_modified(self):
|
||||
stat = os.stat(self.path)
|
||||
return http_date(stat.st_mtime)
|
||||
|
||||
|
||||
class DataLayerView(GZipMixin, BaseDetailView):
|
||||
|
@ -727,8 +725,7 @@ class DataLayerView(GZipMixin, BaseDetailView):
|
|||
with open(path, "rb") as f:
|
||||
# Should not be used in production!
|
||||
response = HttpResponse(f.read(), content_type="application/geo+json")
|
||||
response["Last-Modified"] = http_date(statobj.st_mtime)
|
||||
response["ETag"] = self.etag()
|
||||
response["Last-Modified"] = self.last_modified
|
||||
response["Content-Length"] = statobj.st_size
|
||||
response["Vary"] = "Accept-Encoding"
|
||||
if accepts_gzip and settings.UMAP_GZIP:
|
||||
|
@ -737,7 +734,6 @@ class DataLayerView(GZipMixin, BaseDetailView):
|
|||
|
||||
|
||||
class DataLayerVersion(DataLayerView):
|
||||
|
||||
@property
|
||||
def path(self):
|
||||
return "{root}/{path}".format(
|
||||
|
@ -755,7 +751,7 @@ class DataLayerCreate(FormLessEditMixin, GZipMixin, CreateView):
|
|||
self.object = form.save()
|
||||
# Simple response with only metadatas (including new id)
|
||||
response = simple_json_response(**self.object.metadata)
|
||||
response["ETag"] = self.etag()
|
||||
response["Last-Modified"] = self.last_modified
|
||||
return response
|
||||
|
||||
|
||||
|
@ -768,24 +764,23 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView):
|
|||
# Simple response with only metadatas (client should not reload all data
|
||||
# on save)
|
||||
response = simple_json_response(**self.object.metadata)
|
||||
response["ETag"] = self.etag()
|
||||
response["Last-Modified"] = self.last_modified
|
||||
return response
|
||||
|
||||
def if_match(self):
|
||||
def is_unmodified(self):
|
||||
"""Optimistic concurrency control."""
|
||||
match = True
|
||||
if_match = self.request.META.get("HTTP_IF_MATCH")
|
||||
if if_match:
|
||||
etag = self.etag()
|
||||
if etag != if_match:
|
||||
match = False
|
||||
return match
|
||||
modified = True
|
||||
if_unmodified = self.request.META.get("HTTP_IF_UNMODIFIED_SINCE")
|
||||
if if_unmodified:
|
||||
if self.last_modified != if_unmodified:
|
||||
modified = False
|
||||
return modified
|
||||
|
||||
def post(self, request, *args, **kwargs):
|
||||
self.object = self.get_object()
|
||||
if self.object.map != self.kwargs["map_inst"]:
|
||||
return HttpResponseForbidden()
|
||||
if not self.if_match():
|
||||
if not self.is_unmodified():
|
||||
return HttpResponse(status=412)
|
||||
return super(DataLayerUpdate, self).post(request, *args, **kwargs)
|
||||
|
||||
|
|
Loading…
Reference in a new issue