From a936ad50636ac50fc6e1117dab7aa287de609104 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 13:46:21 +0200 Subject: [PATCH 1/8] mediaapi/writers/download: Add local download support --- .../dendrite/mediaapi/routing/routing.go | 2 +- .../dendrite/mediaapi/writers/download.go | 170 +++++++++++++++++- 2 files changed, 169 insertions(+), 3 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/routing/routing.go b/src/github.com/matrix-org/dendrite/mediaapi/routing/routing.go index 666a102ab..0c1dce6f8 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/routing/routing.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/routing/routing.go @@ -54,7 +54,7 @@ func Setup(servMux *http.ServeMux, httpClient *http.Client, cfg *config.MediaAPI w.Header().Set("Content-Type", "application/json") vars := mux.Vars(req) - writers.Download(w, req, gomatrixserverlib.ServerName(vars["serverName"]), types.MediaID(vars["mediaId"]), cfg, activeRemoteRequests) + writers.Download(w, req, gomatrixserverlib.ServerName(vars["serverName"]), types.MediaID(vars["mediaId"]), cfg, db, activeRemoteRequests) })), ) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index 1d1097d4b..a123ae467 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -15,14 +15,20 @@ package writers import ( + "database/sql" "encoding/json" "fmt" + "io" "net/http" + "os" "regexp" + "strconv" log "github.com/Sirupsen/logrus" "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/mediaapi/config" + "github.com/matrix-org/dendrite/mediaapi/fileutils" + "github.com/matrix-org/dendrite/mediaapi/storage" "github.com/matrix-org/dendrite/mediaapi/types" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" @@ -33,6 +39,13 @@ const mediaIDCharacters = "A-Za-z0-9_=-" // Note: unfortunately regex.MustCompile() cannot be assigned to a const var mediaIDRegex = regexp.MustCompile("[" + mediaIDCharacters + "]+") +// Error types used by downloadRequest.getMediaMetadata +// FIXME: make into types +var ( + errDBQuery = fmt.Errorf("error querying database for media") + errDBNotFound = fmt.Errorf("media not found") +) + // downloadRequest metadata included in or derivable from an download request // https://matrix.org/docs/spec/client_server/r0.2.0.html#get-matrix-media-r0-download-servername-mediaid type downloadRequest struct { @@ -41,7 +54,8 @@ type downloadRequest struct { } // Download implements /download -func Download(w http.ResponseWriter, req *http.Request, origin gomatrixserverlib.ServerName, mediaID types.MediaID, cfg *config.MediaAPI, activeRemoteRequests *types.ActiveRemoteRequests) { +// Files from this server (i.e. origin == cfg.ServerName) are served directly +func Download(w http.ResponseWriter, req *http.Request, origin gomatrixserverlib.ServerName, mediaID types.MediaID, cfg *config.MediaAPI, db *storage.Database, activeRemoteRequests *types.ActiveRemoteRequests) { r := &downloadRequest{ MediaMetadata: &types.MediaMetadata{ MediaID: mediaID, @@ -64,7 +78,10 @@ func Download(w http.ResponseWriter, req *http.Request, origin gomatrixserverlib return } - // doDownload + if resErr := r.doDownload(w, cfg, db, activeRemoteRequests); resErr != nil { + r.jsonErrorResponse(w, *resErr) + return + } } func (r *downloadRequest) jsonErrorResponse(w http.ResponseWriter, res util.JSONResponse) { @@ -101,3 +118,152 @@ func (r *downloadRequest) Validate() *util.JSONResponse { } return nil } + +func (r *downloadRequest) doDownload(w http.ResponseWriter, cfg *config.MediaAPI, db *storage.Database, activeRemoteRequests *types.ActiveRemoteRequests) *util.JSONResponse { + // check if we have a record of the media in our database + mediaMetadata, err := r.getMediaMetadata(db) + if err == nil { + // If we have a record, we can respond from the local file + r.MediaMetadata = mediaMetadata + return r.respondFromLocalFile(w, cfg.AbsBasePath) + } else if err == errDBNotFound { + if r.MediaMetadata.Origin == cfg.ServerName { + // If we do not have a record and the origin is local, the file is not found + r.Logger.WithError(err).Warn("Failed to look up file in database") + return &util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + } + } + // TODO: If we do not have a record and the origin is remote, we need to fetch it and respond with that file + return &util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + } + } + // Another error from the database + r.Logger.WithError(err).WithFields(log.Fields{ + "MediaID": r.MediaMetadata.MediaID, + "Origin": r.MediaMetadata.Origin, + }).Error("Error querying the database.") + return &util.JSONResponse{ + Code: 500, + JSON: jsonerror.InternalServerError(), + } +} + +// getMediaMetadata queries the database for media metadata +func (r *downloadRequest) getMediaMetadata(db *storage.Database) (*types.MediaMetadata, error) { + mediaMetadata, err := db.GetMediaMetadata(r.MediaMetadata.MediaID, r.MediaMetadata.Origin) + if err != nil { + if err == sql.ErrNoRows { + r.Logger.WithFields(log.Fields{ + "Origin": r.MediaMetadata.Origin, + "MediaID": r.MediaMetadata.MediaID, + }).Info("Media not found in database.") + return nil, errDBNotFound + } + r.Logger.WithError(err).WithFields(log.Fields{ + "Origin": r.MediaMetadata.Origin, + "MediaID": r.MediaMetadata.MediaID, + }).Error("Error querying database for media.") + return nil, errDBQuery + } + return mediaMetadata, nil +} + +// respondFromLocalFile reads a file from local storage and writes it to the http.ResponseWriter +// Returns a util.JSONResponse error in case of error +func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePath types.Path) *util.JSONResponse { + filePath, err := fileutils.GetPathFromBase64Hash(r.MediaMetadata.Base64Hash, absBasePath) + if err != nil { + // FIXME: Remove erroneous file from database? + r.Logger.WithError(err).Warn("Failed to get file path from metadata") + return &util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + } + } + file, err := os.Open(filePath) + defer file.Close() + if err != nil { + // FIXME: Remove erroneous file from database? + r.Logger.WithError(err).Warn("Failed to open file") + return &util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + } + } + stat, err := file.Stat() + if err != nil { + // FIXME: Remove erroneous file from database? + r.Logger.WithError(err).Warn("Failed to stat file") + return &util.JSONResponse{ + Code: 404, + JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + } + } + + if r.MediaMetadata.FileSizeBytes > 0 && int64(r.MediaMetadata.FileSizeBytes) != stat.Size() { + r.Logger.WithFields(log.Fields{ + "fileSizeDatabase": r.MediaMetadata.FileSizeBytes, + "fileSizeDisk": stat.Size(), + }).Warn("File size in database and on-disk differ.") + // FIXME: Remove erroneous file from database? + } + + r.Logger.WithFields(log.Fields{ + "MediaID": r.MediaMetadata.MediaID, + "Origin": r.MediaMetadata.Origin, + "UploadName": r.MediaMetadata.UploadName, + "Base64Hash": r.MediaMetadata.Base64Hash, + "FileSizeBytes": r.MediaMetadata.FileSizeBytes, + "Content-Type": r.MediaMetadata.ContentType, + }).Info("Responding with file") + + w.Header().Set("Content-Type", string(r.MediaMetadata.ContentType)) + w.Header().Set("Content-Length", strconv.FormatInt(int64(r.MediaMetadata.FileSizeBytes), 10)) + contentSecurityPolicy := "default-src 'none';" + + " script-src 'none';" + + " plugin-types application/pdf;" + + " style-src 'unsafe-inline';" + + " object-src 'self';" + w.Header().Set("Content-Security-Policy", contentSecurityPolicy) + + if bytesResponded, err := io.Copy(w, file); err != nil { + r.Logger.WithError(err).Warn("Failed to copy from cache") + if bytesResponded == 0 { + return &util.JSONResponse{ + Code: 500, + JSON: jsonerror.NotFound(fmt.Sprintf("Failed to respond with file with media ID %q", r.MediaMetadata.MediaID)), + } + } + // If we have written any data then we have already responded with 200 OK and all we can do is close the connection + // FIXME: close the connection here or just return? + r.closeConnection(w) + } + return nil +} + +func (r *downloadRequest) closeConnection(w http.ResponseWriter) { + r.Logger.WithFields(log.Fields{ + "Origin": r.MediaMetadata.Origin, + "MediaID": r.MediaMetadata.MediaID, + }).Info("Attempting to close the connection.") + hijacker, ok := w.(http.Hijacker) + if ok { + connection, _, hijackErr := hijacker.Hijack() + if hijackErr == nil { + r.Logger.WithFields(log.Fields{ + "Origin": r.MediaMetadata.Origin, + "MediaID": r.MediaMetadata.MediaID, + }).Info("Closing") + connection.Close() + } else { + r.Logger.WithError(hijackErr).WithFields(log.Fields{ + "Origin": r.MediaMetadata.Origin, + "MediaID": r.MediaMetadata.MediaID, + }).Warn("Error trying to hijack and close connection") + } + } +} From a45f008c12bedde4daa8b0a67faf3fba8861e5df Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 14:29:28 +0200 Subject: [PATCH 2/8] mediaapi/storage: Don't leak sql.ErrNoRows out of storage package --- .../dendrite/mediaapi/storage/storage.go | 8 ++- .../dendrite/mediaapi/writers/download.go | 56 ++++--------------- 2 files changed, 18 insertions(+), 46 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/storage/storage.go b/src/github.com/matrix-org/dendrite/mediaapi/storage/storage.go index 4b86967fb..cb27ccc95 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/storage/storage.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/storage/storage.go @@ -50,7 +50,11 @@ func (d *Database) StoreMediaMetadata(mediaMetadata *types.MediaMetadata) error // GetMediaMetadata returns metadata about media stored on this server. // The media could have been uploaded to this server or fetched from another server and cached here. -// Returns sql.ErrNoRows if there is no metadata associated with this media. +// Returns nil metadata if there is no metadata associated with this media. func (d *Database) GetMediaMetadata(mediaID types.MediaID, mediaOrigin gomatrixserverlib.ServerName) (*types.MediaMetadata, error) { - return d.statements.selectMedia(mediaID, mediaOrigin) + mediaMetadata, err := d.statements.selectMedia(mediaID, mediaOrigin) + if err != nil && err == sql.ErrNoRows { + return nil, nil + } + return mediaMetadata, err } diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index a123ae467..f1f74c4cd 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -15,7 +15,6 @@ package writers import ( - "database/sql" "encoding/json" "fmt" "io" @@ -39,13 +38,6 @@ const mediaIDCharacters = "A-Za-z0-9_=-" // Note: unfortunately regex.MustCompile() cannot be assigned to a const var mediaIDRegex = regexp.MustCompile("[" + mediaIDCharacters + "]+") -// Error types used by downloadRequest.getMediaMetadata -// FIXME: make into types -var ( - errDBQuery = fmt.Errorf("error querying database for media") - errDBNotFound = fmt.Errorf("media not found") -) - // downloadRequest metadata included in or derivable from an download request // https://matrix.org/docs/spec/client_server/r0.2.0.html#get-matrix-media-r0-download-servername-mediaid type downloadRequest struct { @@ -121,15 +113,17 @@ func (r *downloadRequest) Validate() *util.JSONResponse { func (r *downloadRequest) doDownload(w http.ResponseWriter, cfg *config.MediaAPI, db *storage.Database, activeRemoteRequests *types.ActiveRemoteRequests) *util.JSONResponse { // check if we have a record of the media in our database - mediaMetadata, err := r.getMediaMetadata(db) - if err == nil { - // If we have a record, we can respond from the local file - r.MediaMetadata = mediaMetadata - return r.respondFromLocalFile(w, cfg.AbsBasePath) - } else if err == errDBNotFound { + mediaMetadata, err := db.GetMediaMetadata(r.MediaMetadata.MediaID, r.MediaMetadata.Origin) + if err != nil { + r.Logger.WithError(err).Error("Error querying the database.") + return &util.JSONResponse{ + Code: 500, + JSON: jsonerror.InternalServerError(), + } + } + if mediaMetadata == nil { if r.MediaMetadata.Origin == cfg.ServerName { // If we do not have a record and the origin is local, the file is not found - r.Logger.WithError(err).Warn("Failed to look up file in database") return &util.JSONResponse{ Code: 404, JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), @@ -141,35 +135,9 @@ func (r *downloadRequest) doDownload(w http.ResponseWriter, cfg *config.MediaAPI JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), } } - // Another error from the database - r.Logger.WithError(err).WithFields(log.Fields{ - "MediaID": r.MediaMetadata.MediaID, - "Origin": r.MediaMetadata.Origin, - }).Error("Error querying the database.") - return &util.JSONResponse{ - Code: 500, - JSON: jsonerror.InternalServerError(), - } -} - -// getMediaMetadata queries the database for media metadata -func (r *downloadRequest) getMediaMetadata(db *storage.Database) (*types.MediaMetadata, error) { - mediaMetadata, err := db.GetMediaMetadata(r.MediaMetadata.MediaID, r.MediaMetadata.Origin) - if err != nil { - if err == sql.ErrNoRows { - r.Logger.WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, - }).Info("Media not found in database.") - return nil, errDBNotFound - } - r.Logger.WithError(err).WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, - }).Error("Error querying database for media.") - return nil, errDBQuery - } - return mediaMetadata, nil + // If we have a record, we can respond from the local file + r.MediaMetadata = mediaMetadata + return r.respondFromLocalFile(w, cfg.AbsBasePath) } // respondFromLocalFile reads a file from local storage and writes it to the http.ResponseWriter From 9606ea28ceba1f6dc319ef6863ef132b5d71ebf3 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 14:30:57 +0200 Subject: [PATCH 3/8] mediaapi/writers/download: Always log origin and mediaID --- .../dendrite/mediaapi/writers/download.go | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index f1f74c4cd..1df286457 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -53,7 +53,10 @@ func Download(w http.ResponseWriter, req *http.Request, origin gomatrixserverlib MediaID: mediaID, Origin: origin, }, - Logger: util.GetLogger(req.Context()), + Logger: util.GetLogger(req.Context()).WithFields(log.Fields{ + "Origin": origin, + "MediaID": mediaID, + }), } // request validation @@ -181,8 +184,6 @@ func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePat } r.Logger.WithFields(log.Fields{ - "MediaID": r.MediaMetadata.MediaID, - "Origin": r.MediaMetadata.Origin, "UploadName": r.MediaMetadata.UploadName, "Base64Hash": r.MediaMetadata.Base64Hash, "FileSizeBytes": r.MediaMetadata.FileSizeBytes, @@ -214,24 +215,15 @@ func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePat } func (r *downloadRequest) closeConnection(w http.ResponseWriter) { - r.Logger.WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, - }).Info("Attempting to close the connection.") + r.Logger.Info("Attempting to close the connection.") hijacker, ok := w.(http.Hijacker) if ok { connection, _, hijackErr := hijacker.Hijack() if hijackErr == nil { - r.Logger.WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, - }).Info("Closing") + r.Logger.Info("Closing") connection.Close() } else { - r.Logger.WithError(hijackErr).WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, - }).Warn("Error trying to hijack and close connection") + r.Logger.WithError(hijackErr).Warn("Error trying to hijack and close connection") } } } From f0c717b0a1793971846e53e0fc1423e67a43197b Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 14:33:49 +0200 Subject: [PATCH 4/8] mediaapi/writers/download: Escalate corrupt db/file cases to errors --- .../dendrite/mediaapi/writers/download.go | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index 1df286457..0df0c30c5 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -148,30 +148,27 @@ func (r *downloadRequest) doDownload(w http.ResponseWriter, cfg *config.MediaAPI func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePath types.Path) *util.JSONResponse { filePath, err := fileutils.GetPathFromBase64Hash(r.MediaMetadata.Base64Hash, absBasePath) if err != nil { - // FIXME: Remove erroneous file from database? - r.Logger.WithError(err).Warn("Failed to get file path from metadata") + r.Logger.WithError(err).Error("Failed to get file path from metadata") return &util.JSONResponse{ - Code: 404, - JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + Code: 500, + JSON: jsonerror.InternalServerError(), } } file, err := os.Open(filePath) defer file.Close() if err != nil { - // FIXME: Remove erroneous file from database? - r.Logger.WithError(err).Warn("Failed to open file") + r.Logger.WithError(err).Error("Failed to open file") return &util.JSONResponse{ - Code: 404, - JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + Code: 500, + JSON: jsonerror.InternalServerError(), } } stat, err := file.Stat() if err != nil { - // FIXME: Remove erroneous file from database? - r.Logger.WithError(err).Warn("Failed to stat file") + r.Logger.WithError(err).Error("Failed to stat file") return &util.JSONResponse{ - Code: 404, - JSON: jsonerror.NotFound(fmt.Sprintf("File with media ID %q does not exist", r.MediaMetadata.MediaID)), + Code: 500, + JSON: jsonerror.InternalServerError(), } } From 5922ad383d5f8613a139a47db8c74c298d0f8735 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 14:52:45 +0200 Subject: [PATCH 5/8] mediaapi/writers/upload: Don't use deprecated sql.ErrNoRows check --- .../dendrite/mediaapi/writers/upload.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go index dc525ee44..88925670d 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go @@ -15,7 +15,6 @@ package writers import ( - "database/sql" "fmt" "io" "net/http" @@ -127,18 +126,26 @@ func (r *uploadRequest) doUpload(reqReader io.Reader, cfg *config.MediaAPI, db * r.MediaMetadata.Base64Hash = hash r.MediaMetadata.MediaID = types.MediaID(hash) + r.Logger = r.Logger.WithField("MediaID", r.MediaMetadata.MediaID) + r.Logger.WithFields(log.Fields{ - "MediaID": r.MediaMetadata.MediaID, - "Origin": r.MediaMetadata.Origin, "Base64Hash": r.MediaMetadata.Base64Hash, "UploadName": r.MediaMetadata.UploadName, "FileSizeBytes": r.MediaMetadata.FileSizeBytes, - "Content-Type": r.MediaMetadata.ContentType, + "ContentType": r.MediaMetadata.ContentType, }).Info("File uploaded") // check if we already have a record of the media in our database and if so, we can remove the temporary directory mediaMetadata, err := db.GetMediaMetadata(r.MediaMetadata.MediaID, r.MediaMetadata.Origin) - if err == nil { + if err != nil { + r.Logger.WithError(err).Error("Error querying the database.") + return &util.JSONResponse{ + Code: 500, + JSON: jsonerror.InternalServerError(), + } + } + + if mediaMetadata != nil { r.MediaMetadata = mediaMetadata fileutils.RemoveDir(tmpDir, r.Logger) return &util.JSONResponse{ @@ -147,8 +154,6 @@ func (r *uploadRequest) doUpload(reqReader io.Reader, cfg *config.MediaAPI, db * ContentURI: fmt.Sprintf("mxc://%s/%s", cfg.ServerName, r.MediaMetadata.MediaID), }, } - } else if err != sql.ErrNoRows { - r.Logger.WithError(err).WithField("MediaID", r.MediaMetadata.MediaID).Warn("Failed to query database") } // TODO: generate thumbnails From e33438a37ec469ee0509afcd0b446b5ed3bbb5a2 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 14:54:10 +0200 Subject: [PATCH 6/8] mediaapi/writers/upload: Add standard fields to logger So that they apply to all log messages thereafter --- .../matrix-org/dendrite/mediaapi/writers/upload.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go index 88925670d..dabd50073 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/upload.go @@ -86,7 +86,7 @@ func parseAndValidateRequest(req *http.Request, cfg *config.MediaAPI) (*uploadRe ContentType: types.ContentType(req.Header.Get("Content-Type")), UploadName: types.Filename(url.PathEscape(req.FormValue("filename"))), }, - Logger: util.GetLogger(req.Context()), + Logger: util.GetLogger(req.Context()).WithField("Origin", cfg.ServerName), } if resErr := r.Validate(cfg.MaxFileSizeBytes); resErr != nil { @@ -98,10 +98,9 @@ func parseAndValidateRequest(req *http.Request, cfg *config.MediaAPI) (*uploadRe func (r *uploadRequest) doUpload(reqReader io.Reader, cfg *config.MediaAPI, db *storage.Database) *util.JSONResponse { r.Logger.WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, "UploadName": r.MediaMetadata.UploadName, "FileSizeBytes": r.MediaMetadata.FileSizeBytes, - "Content-Type": r.MediaMetadata.ContentType, + "ContentType": r.MediaMetadata.ContentType, }).Info("Uploading file") // The file data is hashed and the hash is used as the MediaID. The hash is useful as a @@ -111,8 +110,6 @@ func (r *uploadRequest) doUpload(reqReader io.Reader, cfg *config.MediaAPI, db * hash, bytesWritten, tmpDir, err := fileutils.WriteTempFile(reqReader, cfg.MaxFileSizeBytes, cfg.AbsBasePath) if err != nil { r.Logger.WithError(err).WithFields(log.Fields{ - "Origin": r.MediaMetadata.Origin, - "MediaID": r.MediaMetadata.MediaID, "MaxFileSizeBytes": cfg.MaxFileSizeBytes, }).Warn("Error while transferring file") fileutils.RemoveDir(tmpDir, r.Logger) From bd96d99a3aa3d53c31140415b0eff81e7ee92da9 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 15:08:00 +0200 Subject: [PATCH 7/8] mediaapi/writers/download: 500 ISE if disk and db file size differ --- .../matrix-org/dendrite/mediaapi/writers/download.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index 0df0c30c5..c67e0a4eb 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -177,7 +177,10 @@ func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePat "fileSizeDatabase": r.MediaMetadata.FileSizeBytes, "fileSizeDisk": stat.Size(), }).Warn("File size in database and on-disk differ.") - // FIXME: Remove erroneous file from database? + return &util.JSONResponse{ + Code: 500, + JSON: jsonerror.InternalServerError(), + } } r.Logger.WithFields(log.Fields{ From 2e013e3408e0b48aacbdb1e78021ee56d6ed996a Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Wed, 31 May 2017 15:39:19 +0200 Subject: [PATCH 8/8] mediaapi/writers/download: Remove unnecessary closeConnection() --- .../dendrite/mediaapi/writers/download.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go index c67e0a4eb..9f66409c5 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/writers/download.go @@ -208,22 +208,7 @@ func (r *downloadRequest) respondFromLocalFile(w http.ResponseWriter, absBasePat } } // If we have written any data then we have already responded with 200 OK and all we can do is close the connection - // FIXME: close the connection here or just return? - r.closeConnection(w) + return nil } return nil } - -func (r *downloadRequest) closeConnection(w http.ResponseWriter) { - r.Logger.Info("Attempting to close the connection.") - hijacker, ok := w.(http.Hijacker) - if ok { - connection, _, hijackErr := hijacker.Hijack() - if hijackErr == nil { - r.Logger.Info("Closing") - connection.Close() - } else { - r.Logger.WithError(hijackErr).Warn("Error trying to hijack and close connection") - } - } -}