From 52c2cd03a1612a477d91bbb23ea5ea0b7fef801b Mon Sep 17 00:00:00 2001
From: GraceDB <gracedb@gracedb-dev2.ligo.uwm.edu>
Date: Fri, 13 Jul 2018 08:22:23 -0500
Subject: [PATCH] Update to event file serving
Updated event static file serving to use X-Sendfile with Apache.
This is faster, more efficient, and seems to resolve the issue
with FITS file downloads from the web browser:
https://bugs.ligo.org/redmine/issues/6194
---
gracedb/core/http.py | 5 +
gracedb/events/api/urls.py | 4 +-
gracedb/events/api/views.py | 95 ++-----------------
gracedb/events/buildVOEvent.py | 4 +-
gracedb/events/models.py | 6 +-
gracedb/events/urls.py | 5 +-
gracedb/events/view_logic.py | 2 +-
gracedb/events/views.py | 19 +++-
.../templates/gracedb/event_detail_script.js | 9 +-
gracedb/templates/gracedb/event_filelist.html | 2 +-
10 files changed, 48 insertions(+), 103 deletions(-)
diff --git a/gracedb/core/http.py b/gracedb/core/http.py
index 1b464e09a..5ba5a034c 100644
--- a/gracedb/core/http.py
+++ b/gracedb/core/http.py
@@ -37,6 +37,11 @@ def serve_file(file_path, ResponseClass=HttpResponse):
response.content_type = content_type
response['Content-Type'] = content_type
+ # Set encoding (again in both places)
+ if encoding is not None:
+ response.encoding = encoding
+ response['Content-Encoding'] = encoding
+
# For binary files, add the file as an attachment (direct download instead
# of opening in browser window)
if content_type == "application/octet-stream":
diff --git a/gracedb/events/api/urls.py b/gracedb/events/api/urls.py
index 016817ccf..6a46cdb6d 100644
--- a/gracedb/events/api/urls.py
+++ b/gracedb/events/api/urls.py
@@ -89,6 +89,6 @@ urlpatterns = [
url(r'^performance/$', PerformanceInfo.as_view(), name='performance-info'),
# Legacy
- url(r'^event/(?P<graceid>\w[\d]+)/files/(?P<filename>.+)?$', download,
- name="download2"),
+ url(r'^event/(?P<graceid>\w[\d]+)/files/(?P<filename>.+)?$',
+ Files.as_view(), name="file-download"),
]
diff --git a/gracedb/events/api/views.py b/gracedb/events/api/views.py
index 033d4855b..b917ede48 100644
--- a/gracedb/events/api/views.py
+++ b/gracedb/events/api/views.py
@@ -31,6 +31,7 @@ from ..permission_utils import user_has_perm, filter_events_for_user, \
from .throttles import EventCreationThrottle, AnnotationThrottle
from core.vfile import VersionedFile
+from core.http import check_and_serve_file
from guardian.models import GroupObjectPermission
@@ -1570,72 +1571,6 @@ class GracedbRoot(APIView):
"voevent-types" : dict(VOEvent.VOEVENT_TYPE_CHOICES),
})
-##################################################################
-# Old. Must support this.
-def download(request, graceid, filename=""):
- # Do not filename to be None. That messes up later os.path.join
- filename = filename or ""
-
- try:
- event = Event.getByGraceid(graceid)
- if not user_has_perm(request.user, 'view', event):
- return HttpResponseForbidden("Forbidden")
- except Event.DoesNotExist:
- return HttpResponseNotFound("Event not found")
-
- # If the user is external, check for authorization
- if is_external(request.user):
- if not check_external_file_access(event, filename):
- msg = "You do not have permission to view this file."
- return HttpResponseForbidden(msg)
-
- filepath = os.path.join(event.datadir(), filename)
-
- if not os.path.exists(filepath):
- response = HttpResponseNotFound("File does not exist")
- elif not os.access(filepath, os.R_OK):
- response = HttpResponseNotFound("File not readable")
- elif os.path.isfile(filepath):
- # get an actual file.
- content_type, encoding = VersionedFile.guess_mimetype(filepath)
- content_type = content_type or "application/octet-stream"
- # XXX encoding should probably not be ignored.
-
- # Get a pretty filename. Just strip off version info.
- # XXX This will break if you change the file-version naming convention
- # (by, for instance, using a different delimiter than a comma).
- try:
- ind = filename.index(',')
- pretty_filename = filename[:ind]
- except ValueError:
- pretty_filename = filename
-
- response = HttpResponse(open(filepath, "r"), content_type=content_type)
- if content_type == "application/octet-stream":
- #response['Content-Disposition'] = 'attachment; filename=%s' % os.path.basename(filename)
- response['Content-Disposition'] = 'attachment; filename=%s' % pretty_filename
- else:
- response['Content-Disposition'] = 'inline; filename=%s' % pretty_filename
- if encoding is not None:
- response['Content-Encoding'] = encoding
- elif not filename:
- # Get list of files w/urls.
- rv = {}
- filepath = event.datadir()
- for dirname, dirnames, filenames in os.walk(filepath):
- dirname = dirname[len(filepath):] # cut off base event dir path
- for filename in filenames:
- # relative path from root of event data dir
- filename = os.path.join(dirname, filename)
- rv[filename] = django_reverse(download, args=[graceid, filename])
-
- response = HttpResponse(json.dumps(rv), content_type="application/json")
- elif os.path.isdir(filepath):
- response = HttpResponseForbidden("%s is a directory" % filename)
- else:
- response = HttpResponseServerError("Should not happen.")
-
- return response
class Files(APIView):
"""Files Resource"""
@@ -1653,28 +1588,17 @@ class Files(APIView):
filepath = os.path.join(event.datadir(), filename)
- if not os.path.exists(filepath):
- response = HttpResponseNotFound("File does not exist")
- elif not os.access(filepath, os.R_OK):
- response = HttpResponseNotFound("File not readable")
- elif os.path.isfile(filepath):
- # get an actual file.
- # If the user is external, check for authorization
+ # Check permissions for external users
+ if filename and os.path.isdir(filepath):
+ # XXX Really?
+ response = HttpResponseForbidden("%s is a directory" % filename)
+ elif filename:
if is_external(request.user):
if not check_external_file_access(event, filename):
msg = "You do not have permission to view this file."
return HttpResponseForbidden(msg)
-
- content_type, encoding = VersionedFile.guess_mimetype(filepath)
- content_type = content_type or "application/octet-stream"
- # XXX encoding should probably not be ignored.
- response = HttpResponse(open(filepath, "r"), content_type=content_type)
- if content_type == "application/octet-stream":
- # Double quotes are required to avoid MUTLIPLE_CONTENT_DISPOSITION error
- # in Chrome when the filename contains a comma.
- response['Content-Disposition'] = 'attachment; filename="%s"' % os.path.basename(filename)
- if encoding is not None:
- response['Content-Encoding'] = encoding
+ response = check_and_serve_file(request, filepath,
+ ResponseClass=Response)
elif not filename:
# Get list of files w/urls.
rv = {}
@@ -1714,9 +1638,6 @@ class Files(APIView):
request=request),
})
response = Response(rv)
- elif os.path.isdir(filepath):
- # XXX Really?
- response = HttpResponseForbidden("%s is a directory" % filename)
else:
response = HttpResponseServerError("Should not happen.")
diff --git a/gracedb/events/buildVOEvent.py b/gracedb/events/buildVOEvent.py
index 918da4144..66960ffab 100644
--- a/gracedb/events/buildVOEvent.py
+++ b/gracedb/events/buildVOEvent.py
@@ -291,9 +291,9 @@ def buildVOEvent(event, serial_number, voevent_type, request=None, skymap_filena
g = Group('GW_SKYMAP', skymap_type)
# shib urls.
- shib_fits_skymap_url = get_url(request, objid, "file", fits_name)
+ shib_fits_skymap_url = get_url(request, objid, "file-download", fits_name)
if img_name:
- shib_png_skymap_url = get_url(request, objid, "file", img_name)
+ shib_png_skymap_url = get_url(request, objid, "file-download", img_name)
# x509 urls. Hafta specify the api namespace.
x509_fits_skymap_url = get_url(request, objid, "x509:files", fits_name)
diff --git a/gracedb/events/models.py b/gracedb/events/models.py
index c620faab5..c0b12f6cf 100644
--- a/gracedb/events/models.py
+++ b/gracedb/events/models.py
@@ -436,7 +436,8 @@ class EventLog(AutoIncrementModel):
actual_filename = self.filename
if self.file_version >= 0:
actual_filename += ',%d' % self.file_version
- return reverse('file', args=[self.event.graceid(), actual_filename])
+ return reverse('file-download', args=[self.event.graceid(),
+ actual_filename])
else:
return None
@@ -1143,7 +1144,8 @@ class VOEvent(models.Model):
actual_filename = self.filename
if self.file_version >= 0:
actual_filename += ',%d' % self.file_version
- return reverse('file', args=[self.event.graceid(), actual_filename])
+ return reverse('file-download', args=[self.event.graceid(),
+ actual_filename])
else:
return None
diff --git a/gracedb/events/urls.py b/gracedb/events/urls.py
index 4825c55f4..b27534f08 100644
--- a/gracedb/events/urls.py
+++ b/gracedb/events/urls.py
@@ -6,7 +6,6 @@ from django.conf.urls import url
from . import views
#import django.views.generic.list_detail
-from .api.views import download
urlpatterns = [
url(r'^$', views.index, name="home-events"),
@@ -26,8 +25,8 @@ urlpatterns = [
name="modify_signoff"),
url(r'^(?P<graceid>[GEHMT]\d+)/files/$', views.file_list,
name="file_list"),
- url(r'^(?P<graceid>[GEHMT]\d+)/files/(?P<filename>.*)$', download,
- name="file"),
+ url(r'^(?P<graceid>[GEHMT]\d+)/files/(?P<filename>.*)$',
+ views.file_download, name="file-download"),
url(r'^(?P<graceid>[GEHMT]\d+)/log/(?P<num>([\d]*|preview))$',
views.logentry, name="logentry"),
url(r'^(?P<graceid>[GEHMT]\d+)/embblog/(?P<num>([\d]*|preview))$',
diff --git a/gracedb/events/view_logic.py b/gracedb/events/view_logic.py
index 945686c01..2634c3fd0 100644
--- a/gracedb/events/view_logic.py
+++ b/gracedb/events/view_logic.py
@@ -153,7 +153,7 @@ def _createEventFromForm(request, form):
# XXX This reverse will give the web-interface URL, not the REST URL.
# This could be a problem if anybody ever tries to use it.
issueAlert(event,
- request.build_absolute_uri(reverse("file", args=[event.graceid(),f.name])),
+ request.build_absolute_uri(reverse("file-download", args=[event.graceid(),f.name])),
request.build_absolute_uri(reverse("view", args=[event.graceid()])),
eventToDict(event, request=request))
except Exception, e:
diff --git a/gracedb/events/views.py b/gracedb/events/views.py
index f41a64a66..693056599 100644
--- a/gracedb/events/views.py
+++ b/gracedb/events/views.py
@@ -6,6 +6,7 @@ from django.template import RequestContext
from django.urls import reverse
from django.shortcuts import render
+from core.http import check_and_serve_file
from .models import Event, Group, EventLog, Label, Tag, Pipeline, Search, GrbEvent
from .models import EMGroup, Signoff
from .forms import CreateEventForm, EventSearchForm, SimpleSearchForm, SignoffForm
@@ -14,7 +15,8 @@ from django.contrib.auth.models import User, Permission
from django.contrib.auth.models import Group as AuthGroup
from django.contrib.contenttypes.models import ContentType
from .permission_utils import filter_events_for_user, user_has_perm
-from .permission_utils import internal_user_required, is_external
+from .permission_utils import internal_user_required, is_external, \
+ check_external_file_access
from guardian.models import GroupObjectPermission
from .view_logic import _createEventFromForm
@@ -866,6 +868,21 @@ def file_list(request, event):
return render(request, 'gracedb/event_filelist.html', context=context)
+
+@event_and_auth_required
+def file_download(request, event, filename):
+
+ # If the user is external, check for authorization
+ if is_external(request.user):
+ if not check_external_file_access(event, filename):
+ msg = "You do not have permission to view this file."
+ return HttpResponseForbidden(msg)
+
+ file_path = os.path.join(event.datadir, filename)
+ return check_and_serve_file(request, file_path,
+ ResponseClass=HttpResponse)
+
+
# A view to modify the GroupObjectPermissions for an event.
# This is very non-RESTful. If the action is 'expose', you
# give the group both view and change permissions on the event.
diff --git a/gracedb/templates/gracedb/event_detail_script.js b/gracedb/templates/gracedb/event_detail_script.js
index 1c05477f8..879cb5c45 100644
--- a/gracedb/templates/gracedb/event_detail_script.js
+++ b/gracedb/templates/gracedb/event_detail_script.js
@@ -161,7 +161,8 @@ var eventLogListUrl = '{% url "shib:eventlog-list" object.graceid %}';
var eventLogSaveUrl = '{% url "logentry" object.graceid "" %}';
var embbEventLogListUrl = '{% url "shib:embbeventlog-list" object.graceid %}';
var emObservationListUrl = '{% url "shib:emobservation-list" object.graceid %}';
-var skymapJsonUrl = '{% url "file" object.graceid "" %}';
+var fileDownloadUrl = '{% url "file-download" object.graceid "FAKE_FILE_NAME" %}';
+var skymapJsonUrl = '{% url "shib:files" object.graceid "" %}';
var skymapViewerUrl = '{{ SKYMAP_VIEWER_SERVICE_URL }}';
// This little list determines the priority ordering of the digest sections.
@@ -636,7 +637,7 @@ require([
// Putting this in the innerHTML allows users to create comments in HTML.
// Whereas, inserting the comment with the put selector escapes it.
commentDiv.innerHTML += value + ' ';
- if (object.filename) put(commentDiv, 'a[href=$]', object.file, object.filename);
+ if (object.filename) put(commentDiv, 'a[href=$]', fileDownloadUrl.replace("FAKE_FILE_NAME", object.filename + "," + object.file_version), object.filename);
// Branson, 3/3/15
//if (object.filename == 'skymap.json') {
var isItJson = object.filename.indexOf(".json");
@@ -796,7 +797,7 @@ require([
// Putting this in the innerHTML allows users to create comments in HTML.
// Whereas, inserting the comment with the put selector escapes it.
commentDiv.innerHTML += value + ' ';
- if (object.filename) put(commentDiv, 'a[href=$]', object.file, object.filename);
+ if (object.filename) put(commentDiv, 'a[href=$]', fileDownloadUrl.replace("FAKE_FILE_NAME", object.filename + "," + object.file_version), object.filename);
// Create tag-related features
var tagButtonContainer = put(commentDiv, 'div.tagButtonContainerClass');
// For each existing tag on a log message, we will make a little widget
@@ -888,7 +889,7 @@ require([
// Putting this in the innerHTML allows users to create comments in HTML.
// Whereas, inserting the comment with the put selector escapes it.
commentDiv.innerHTML += value + ' ';
- if (object.filename) put(commentDiv, 'a[href=$]', object.file, object.filename);
+ if (object.filename) put(commentDiv, 'a[href=$]', fileDownloadUrl.replace("FAKE_FILE_NAME", object.filename + "," + object.file_version), object.filename);
// Create tag-related features
var tagButtonContainer = put(commentDiv, 'div.tagButtonContainerClass');
// For each existing tag on a log message, we will make a little widget
diff --git a/gracedb/templates/gracedb/event_filelist.html b/gracedb/templates/gracedb/event_filelist.html
index 1f413294a..b6277067e 100644
--- a/gracedb/templates/gracedb/event_filelist.html
+++ b/gracedb/templates/gracedb/event_filelist.html
@@ -8,7 +8,7 @@
<ul>
{% for filename in file_list %}
-<li><a href="{% url "file" graceid filename %}">{{ filename }}</a></li>
+<li><a href="{% url "file-download" graceid filename %}">{{ filename }}</a></li>
{% endfor %}
</ul>
--
GitLab