From 46122fe6b459574e132bcee32ebe30913a5b09bc Mon Sep 17 00:00:00 2001 From: Farid AYOUJIL Date: Mon, 1 Feb 2021 20:56:29 +0100 Subject: [PATCH 1/4] Add Content-Length header to HEAD requests This change adds the header Content-Length to HEAD HTTP requests. The previous behaviour was blocking some Windows executables (i.e bitsadmin.exe) from downloading files hosted in Gitea. This along with PR #14541, makes the web server compliant with HTTP RFC 2616 which states "The methods GET and HEAD MUST be supported by all general-purpose servers" and "The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response." This should also respond to issues #8030 and #14532. --- routers/repo/download.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/routers/repo/download.go b/routers/repo/download.go index f04dac6aa5146..f210401dba1e8 100644 --- a/routers/repo/download.go +++ b/routers/repo/download.go @@ -8,6 +8,7 @@ package repo import ( "fmt" "io" + "io/ioutil" "path" "strings" @@ -21,16 +22,15 @@ import ( // ServeData download file from io.Reader func ServeData(ctx *context.Context, name string, reader io.Reader) error { - buf := make([]byte, 1024) - n, err := reader.Read(buf) + content, err := ioutil.ReadAll(reader) if err != nil && err != io.EOF { return err } - if n >= 0 { - buf = buf[:n] - } + length := len(content) + buf := content[:1024] ctx.Resp.Header().Set("Cache-Control", "public,max-age=86400") + ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", length)) name = path.Base(name) // Google Chrome dislike commas in filenames, so let's change it to a space @@ -56,11 +56,7 @@ func ServeData(ctx *context.Context, name string, reader io.Reader) error { ctx.Resp.Header().Set("Access-Control-Expose-Headers", "Content-Disposition") } - _, err = ctx.Resp.Write(buf) - if err != nil { - return err - } - _, err = io.Copy(ctx.Resp, reader) + _, err = ctx.Resp.Write(content) return err } From b2e00aa2361331e4fba85d7ab96a67e1b67c6a42 Mon Sep 17 00:00:00 2001 From: Farid AYOUJIL Date: Mon, 1 Feb 2021 22:36:39 +0100 Subject: [PATCH 2/4] This change adds the header Content-Length to HEAD HTTP requests Pass the Size of the content as a parameter to ServeData() instead of calculating it using ioutil.ReadAll(reader) --> this call is dangerous and can result in a denial of service. --- routers/repo/attachment.go | 2 +- routers/repo/download.go | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/routers/repo/attachment.go b/routers/repo/attachment.go index 5b699abc8d11d..5df9cdbf12a62 100644 --- a/routers/repo/attachment.go +++ b/routers/repo/attachment.go @@ -152,7 +152,7 @@ func GetAttachment(ctx *context.Context) { return } - if err = ServeData(ctx, attach.Name, fr); err != nil { + if err = ServeData(ctx, attach.Name, attach.Size, fr); err != nil { ctx.ServerError("ServeData", err) return } diff --git a/routers/repo/download.go b/routers/repo/download.go index f210401dba1e8..c148ba86464b8 100644 --- a/routers/repo/download.go +++ b/routers/repo/download.go @@ -21,16 +21,18 @@ import ( ) // ServeData download file from io.Reader -func ServeData(ctx *context.Context, name string, reader io.Reader) error { - content, err := ioutil.ReadAll(reader) +func ServeData(ctx *context.Context, name string, size int64, reader io.Reader) error { + buf := make([]byte, 1024) + n, err := reader.Read(buf) if err != nil && err != io.EOF { return err } - length := len(content) - buf := content[:1024] + if n >= 0 { + buf = buf[:n] + } ctx.Resp.Header().Set("Cache-Control", "public,max-age=86400") - ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", length)) + ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", size)) name = path.Base(name) // Google Chrome dislike commas in filenames, so let's change it to a space @@ -56,7 +58,11 @@ func ServeData(ctx *context.Context, name string, reader io.Reader) error { ctx.Resp.Header().Set("Access-Control-Expose-Headers", "Content-Disposition") } - _, err = ctx.Resp.Write(content) + _, err = ctx.Resp.Write(buf) + if err != nil { + return err + } + _, err = io.Copy(ctx.Resp, reader) return err } @@ -72,7 +78,7 @@ func ServeBlob(ctx *context.Context, blob *git.Blob) error { } }() - return ServeData(ctx, ctx.Repo.TreePath, dataRc) + return ServeData(ctx, ctx.Repo.TreePath, blob.Size(), dataRc) } // ServeBlobOrLFS download a git.Blob redirecting to LFS if necessary @@ -101,7 +107,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob) error { log.Error("ServeBlobOrLFS: Close: %v", err) } }() - return ServeData(ctx, ctx.Repo.TreePath, lfsDataRc) + return ServeData(ctx, ctx.Repo.TreePath, meta.Size, lfsDataRc) } return ServeBlob(ctx, blob) From d94cbd1a30d578c84b5bb4dd69aee437dddd11ce Mon Sep 17 00:00:00 2001 From: Farid AYOUJIL Date: Mon, 1 Feb 2021 22:43:07 +0100 Subject: [PATCH 3/4] Add Content-Length header to HEAD requests Quick fix for imported dependency not used. --- routers/repo/download.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/repo/download.go b/routers/repo/download.go index c148ba86464b8..2092b41b8512f 100644 --- a/routers/repo/download.go +++ b/routers/repo/download.go @@ -8,7 +8,6 @@ package repo import ( "fmt" "io" - "io/ioutil" "path" "strings" From fbe554f40042e4ca78d7c00a6eaada7f9a959c2a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 5 Feb 2021 19:56:03 +0100 Subject: [PATCH 4/4] Check if size is positiv int ... Co-authored-by: zeripath --- routers/repo/download.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/routers/repo/download.go b/routers/repo/download.go index 2092b41b8512f..50f893690b1a8 100644 --- a/routers/repo/download.go +++ b/routers/repo/download.go @@ -31,7 +31,11 @@ func ServeData(ctx *context.Context, name string, size int64, reader io.Reader) } ctx.Resp.Header().Set("Cache-Control", "public,max-age=86400") - ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", size)) + if size >= 0 { + ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", size)) + } else { + log.Error("ServeData called to serve data: %s with size < 0: %d", name, size) + } name = path.Base(name) // Google Chrome dislike commas in filenames, so let's change it to a space