Skip to content

Commit 2ee853c

Browse files
wxiaoguangwxiaoguang
committed
fix error handling
1 parent 7466f24 commit 2ee853c

File tree

6 files changed

+32
-44
lines changed

6 files changed

+32
-44
lines changed

‎modules/httplib/serve.go

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func ServeSetHeaders(w http.ResponseWriter, opts *ServeHeaderOptions) {
7676
}
7777

7878
// ServeData download file from io.Reader
79-
func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, filePath string, mineBuf []byte) error {
79+
func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, filePath string, mineBuf []byte) {
8080
// do not set "Content-Length", because the length could only be set by callers, and it needs to support range requests
8181
opts := &ServeHeaderOptions{
8282
Filename: path.Base(filePath),
@@ -129,23 +129,21 @@ func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, filePath stri
129129
}
130130

131131
ServeSetHeaders(w, opts)
132-
return nil
133132
}
134133

135134
const mimeDetectionBufferLen = 1024
136135

137-
func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath string, size int64, reader io.Reader) error {
136+
func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath string, size int64, reader io.Reader) {
138137
buf := make([]byte, mimeDetectionBufferLen)
139138
n, err := util.ReadAtMost(reader, buf)
140139
if err != nil {
141-
return err
140+
http.Error(w, "serve content: unable to pre-read", http.StatusRequestedRangeNotSatisfiable)
141+
return
142142
}
143143
if n >= 0 {
144144
buf = buf[:n]
145145
}
146-
if err = setServeHeadersByFile(r, w, filePath, buf); err != nil {
147-
return err
148-
}
146+
setServeHeadersByFile(r, w, filePath, buf)
149147

150148
// reset the reader to the beginning
151149
reader = io.MultiReader(bytes.NewReader(buf), reader)
@@ -157,8 +155,8 @@ func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath strin
157155
if size >= 0 {
158156
w.Header().Set("Content-Length", strconv.FormatInt(size, 10))
159157
}
160-
_, err = io.Copy(w, reader)
161-
return err
158+
_, _ = io.Copy(w, reader) // just like http.ServeContent, not necessary to handle the error
159+
return
162160
}
163161

164162
// do our best to support the minimal "Range" request (no support for multiple range: "Range: bytes=0-50, 100-150")
@@ -178,7 +176,7 @@ func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath strin
178176
}
179177
if err != nil {
180178
http.Error(w, err.Error(), http.StatusRequestedRangeNotSatisfiable)
181-
return err
179+
return
182180
}
183181
end, err := strconv.ParseInt(rangeBytesEnd, 10, 64)
184182
if rangeBytesEnd == "" && found {
@@ -193,36 +191,35 @@ func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath strin
193191
}
194192
if err != nil {
195193
http.Error(w, err.Error(), http.StatusBadRequest)
196-
return err
194+
return
197195
}
198196

199197
partialLength := end - start + 1
200198
w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", start, end, size))
201199
w.Header().Set("Content-Length", strconv.FormatInt(partialLength, 10))
202200
if _, err = io.CopyN(io.Discard, reader, start); err != nil {
203-
return fmt.Errorf("unable to skip first %d bytes: %w", start, err)
201+
http.Error(w, "serve content: unable to skip", http.StatusRequestedRangeNotSatisfiable)
202+
return
204203
}
205204

206205
w.WriteHeader(http.StatusPartialContent)
207-
_, err = io.CopyN(w, reader, partialLength)
208-
return err
206+
_, _ = io.CopyN(w, reader, partialLength) // just like http.ServeContent, not necessary to handle the error
209207
}
210208

211-
func ServeContentByReadSeeker(r *http.Request, w http.ResponseWriter, filePath string, modTime time.Time, reader io.ReadSeeker) error {
209+
func ServeContentByReadSeeker(r *http.Request, w http.ResponseWriter, filePath string, modTime time.Time, reader io.ReadSeeker) {
212210
buf := make([]byte, mimeDetectionBufferLen)
213211
n, err := util.ReadAtMost(reader, buf)
214212
if err != nil {
215-
return err
213+
http.Error(w, "serve content: unable to read", http.StatusInternalServerError)
214+
return
216215
}
217216
if _, err = reader.Seek(0, io.SeekStart); err != nil {
218-
return err
217+
http.Error(w, "serve content: unable to seek", http.StatusInternalServerError)
218+
return
219219
}
220220
if n >= 0 {
221221
buf = buf[:n]
222222
}
223-
if err = setServeHeadersByFile(r, w, filePath, buf); err != nil {
224-
return err
225-
}
223+
setServeHeadersByFile(r, w, filePath, buf)
226224
http.ServeContent(w, r, path.Base(filePath), modTime, reader)
227-
return nil
228225
}

‎modules/httplib/serve_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,11 @@ func TestServeContentByReader(t *testing.T) {
2626
}
2727
reader := strings.NewReader(data)
2828
w := NewMockResponseWriter()
29-
err := ServeContentByReader(r, w, "test", int64(len(data)), reader)
29+
ServeContentByReader(r, w, "test", int64(len(data)), reader)
3030
assert.Equal(t, expectedStatusCode, w.StatusCode)
3131
if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK {
3232
assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length"))
3333
assert.Equal(t, expectedContent, w.BodyBuffer.String())
34-
} else {
35-
assert.Error(t, err)
3634
}
3735
}
3836

@@ -79,7 +77,7 @@ func TestServeContentByReadSeeker(t *testing.T) {
7977
defer seekReader.Close()
8078

8179
w := NewMockResponseWriter()
82-
_ = ServeContentByReadSeeker(r, w, "test", time.Time{}, seekReader)
80+
ServeContentByReadSeeker(r, w, "test", time.Time{}, seekReader)
8381
assert.Equal(t, expectedStatusCode, w.StatusCode)
8482
if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK {
8583
assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length"))

‎routers/api/v1/repo/file.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
173173
}
174174

175175
// OK not cached - serve!
176-
if err := common.ServeContentByReader(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf)); err != nil {
177-
ctx.ServerError("ServeBlob", err)
178-
}
176+
common.ServeContentByReader(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf))
179177
return
180178
}
181179

@@ -189,9 +187,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
189187
return
190188
}
191189

192-
if err := common.ServeContentByReader(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf)); err != nil {
193-
ctx.ServerError("ServeBlob", err)
194-
}
190+
common.ServeContentByReader(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf))
195191
return
196192
} else if err != nil {
197193
ctx.ServerError("GetLFSMetaObjectByOid", err)
@@ -219,9 +215,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
219215
}
220216
defer lfsDataRc.Close()
221217

222-
if err := common.ServeContentByReadSeeker(ctx.Context, ctx.Repo.TreePath, lastModified, lfsDataRc); err != nil {
223-
ctx.ServerError("ServeData", err)
224-
}
218+
common.ServeContentByReadSeeker(ctx.Context, ctx.Repo.TreePath, lastModified, lfsDataRc)
225219
}
226220

227221
func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, entry *git.TreeEntry, lastModified time.Time) {

‎routers/common/serve.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,14 @@ func ServeBlob(ctx *context.Context, blob *git.Blob, lastModified time.Time) err
3030
}
3131
}()
3232

33-
return httplib.ServeContentByReader(ctx.Req, ctx.Resp, ctx.Repo.TreePath, blob.Size(), dataRc)
33+
httplib.ServeContentByReader(ctx.Req, ctx.Resp, ctx.Repo.TreePath, blob.Size(), dataRc)
34+
return nil
3435
}
3536

36-
func ServeContentByReader(ctx *context.Context, filePath string, size int64, reader io.Reader) error {
37-
return httplib.ServeContentByReader(ctx.Req, ctx.Resp, filePath, size, reader)
37+
func ServeContentByReader(ctx *context.Context, filePath string, size int64, reader io.Reader) {
38+
httplib.ServeContentByReader(ctx.Req, ctx.Resp, filePath, size, reader)
3839
}
3940

40-
func ServeContentByReadSeeker(ctx *context.Context, filePath string, modTime time.Time, reader io.ReadSeeker) error {
41-
return httplib.ServeContentByReadSeeker(ctx.Req, ctx.Resp, filePath, modTime, reader)
41+
func ServeContentByReadSeeker(ctx *context.Context, filePath string, modTime time.Time, reader io.ReadSeeker) {
42+
httplib.ServeContentByReadSeeker(ctx.Req, ctx.Resp, filePath, modTime, reader)
4243
}

‎routers/web/repo/attachment.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,7 @@ func ServeAttachment(ctx *context.Context, uuid string) {
153153
}
154154
defer fr.Close()
155155

156-
if err = common.ServeContentByReadSeeker(ctx, attach.Name, attach.CreatedUnix.AsTime(), fr); err != nil {
157-
ctx.ServerError("ServeData", err)
158-
return
159-
}
156+
common.ServeContentByReadSeeker(ctx, attach.Name, attach.CreatedUnix.AsTime(), fr)
160157
}
161158

162159
// GetAttachment serve attachments

‎routers/web/repo/download.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified time.Time
7171
log.Error("ServeBlobOrLFS: Close: %v", err)
7272
}
7373
}()
74-
return common.ServeContentByReadSeeker(ctx, ctx.Repo.TreePath, lastModified, lfsDataRc)
74+
common.ServeContentByReadSeeker(ctx, ctx.Repo.TreePath, lastModified, lfsDataRc)
75+
return nil
7576
}
7677
if err = dataRc.Close(); err != nil {
7778
log.Error("ServeBlobOrLFS: Close: %v", err)

0 commit comments

Comments
 (0)