From 3d519b27734182b9e7c652493af6b8992a7de558 Mon Sep 17 00:00:00 2001 From: mpl Date: Sat, 10 Jun 2017 00:34:37 +0200 Subject: [PATCH] server/camlistored: add inline query parameter, and set Content-Disposition The download handler was setting the Content-Disposition to attachment when the MIME type was "application/octet-stream". That means when e.g. a plain text file (and probably others) is (incorrectly) detected as "application/octet-stream", it is saved to disk instead of being opened as text by the browser, when using the "View Original" button of the web UI. Conversely, when using the "Download" button of the web UI, the file should always be saved to disk, and never be opened by the browser. Therefore, we add the ?inline request parameter to the downloadhandler GET handler to unambiguously request whether the response should have an inline or an attachment Content-Disposition. The View Original button action now always requests an inlined response. The Download button action now detects whether the selection was only about one file, and if yes it requests the file as an attachment (instead of using the POST handler that zips all the requested files). Change-Id: I4bbc42f17fb4ea0aff9e17056e80156611d4b419 --- pkg/server/download.go | 60 +++++++++++++++---- .../ui/goui/downloadbutton/downloadbutton.go | 6 ++ server/camlistored/ui/index.js | 2 +- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/pkg/server/download.go b/pkg/server/download.go index dd3a4165b..651b4388a 100644 --- a/pkg/server/download.go +++ b/pkg/server/download.go @@ -18,6 +18,7 @@ package server import ( "archive/zip" + "bytes" "context" "errors" "fmt" @@ -30,6 +31,7 @@ import ( "regexp" "strings" "time" + "unicode/utf8" "go4.org/readerutil" "perkeep.org/internal/httputil" @@ -265,8 +267,10 @@ func fileInfoPacked(ctx context.Context, sh *search.Handler, src blob.Fetcher, r // Creates a zip archive of the provided files and serves it in the response. // // GET: -// / +// /[?inline=1] // Serves the file described by the requested file schema blobref. +// if inline=1 the Content-Disposition of the response is set to inline, and +// otherwise it set to attachment. func (dh *DownloadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Method == "POST" { dh.serveZip(w, r) @@ -315,23 +319,39 @@ func (dh *DownloadHandler) ServeFile(w http.ResponseWriter, r *http.Request, fil h := w.Header() h.Set("Content-Length", fmt.Sprint(fi.size)) h.Set("Expires", time.Now().Add(oneYear).Format(http.TimeFormat)) - h.Set("Content-Type", fi.mime) if packed { h.Set("X-Camlistore-Packed", "1") } - if fi.mime == "application/octet-stream" { - // Chrome seems to silently do nothing on - // application/octet-stream unless this is set. - // Maybe it's confused by lack of URL it recognizes - // along with lack of mime type? - fileName := fi.name - if fileName == "" { - fileName = "file-" + file.String() + ".dat" + fileName := func(ext string) string { + if fi.name != "" { + return fi.name } - w.Header().Set("Content-Disposition", "attachment; filename="+fileName) + return "file-" + file.String() + ext } + if r.FormValue("inline") == "1" { + // TODO(mpl): investigate why at least text files have an incorrect MIME. + if fi.mime == "application/octet-stream" { + // Since e.g. plain text files are seen as "application/octet-stream", we force + // check for that, so we can have the browser display them as text if they are + // indeed actually text. + text, err := isText(fi.rs) + if err != nil { + // TODO: https://perkeep.org/issues/1060 + httputil.ServeError(w, r, fmt.Errorf("cannot verify MIME type of file: %v", err)) + return + } + if text { + fi.mime = "text/plain" + } + } + h.Set("Content-Disposition", "inline") + } else { + w.Header().Set("Content-Disposition", "attachment; filename="+fileName(".dat")) + } + h.Set("Content-Type", fi.mime) + if r.Method == "HEAD" && r.FormValue("verifycontents") != "" { vbr, ok := blob.Parse(r.FormValue("verifycontents")) if !ok { @@ -351,6 +371,24 @@ func (dh *DownloadHandler) ServeFile(w http.ResponseWriter, r *http.Request, fil http.ServeContent(w, r, "", time.Now(), fi.rs) } +// isText reports whether the first MB read from rs is valid UTF-8 text. +func isText(rs io.ReadSeeker) (ok bool, err error) { + defer func() { + if _, seekErr := rs.Seek(0, io.SeekStart); seekErr != nil { + if err == nil { + err = seekErr + } + } + }() + var buf bytes.Buffer + if _, err := io.CopyN(&buf, rs, 1e6); err != nil { + if err != io.EOF { + return false, err + } + } + return utf8.Valid(buf.Bytes()), nil +} + // statFiles stats the given refs and returns an error if any one of them is not // found. // It is the responsibility of the caller to check that dh.Fetcher is a diff --git a/server/camlistored/ui/goui/downloadbutton/downloadbutton.go b/server/camlistored/ui/goui/downloadbutton/downloadbutton.go index 9c46b3830..2de69b2f2 100644 --- a/server/camlistored/ui/goui/downloadbutton/downloadbutton.go +++ b/server/camlistored/ui/goui/downloadbutton/downloadbutton.go @@ -135,6 +135,12 @@ func (d DownloadItemsBtnDef) downloadSelection() error { fileRefs = append(fileRefs, ref.String()) } + if len(fileRefs) < 2 { + // Do not ask for a zip if we only want one file + dom.GetWindow().Open(fmt.Sprintf("%s/%s", downloadPrefix, fileRefs[0]), "", "") + return nil + } + el := dom.GetWindow().Document().CreateElement("input") input := el.(*dom.HTMLInputElement) input.Type = "text" diff --git a/server/camlistored/ui/index.js b/server/camlistored/ui/index.js index a8de69fde..4ca9da180 100644 --- a/server/camlistored/ui/index.js +++ b/server/camlistored/ui/index.js @@ -1097,7 +1097,7 @@ cam.IndexPage = React.createClass({ fileName = goog.string.subs('/%s', rm.file.fileName); } - var downloadUrl = goog.string.subs('%s%s%s', this.props.config.downloadHelper, rm.blobRef, fileName); + var downloadUrl = goog.string.subs('%s%s%s?inline=1', this.props.config.downloadHelper, rm.blobRef, fileName); return React.DOM.button( { key:'viewSelection',