diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt
index 3aad357156730e7efd20bfeb68fea4df13250f7e..e75c56c3d9725ce34fee2b2369610f968da1b53f 100644
--- a/workhorse/_support/lint_last_known_acceptable.txt
+++ b/workhorse/_support/lint_last_known_acceptable.txt
@@ -99,8 +99,6 @@ internal/imageresizer/image_resizer_caching.go:6:1: ST1000: at least one file in
internal/proxy/proxy.go:142:14: SA6002: argument should be pointer-like to avoid allocations (staticcheck)
internal/senddata/contentprocessor/contentprocessor.go:136:35: response body must be closed (bodyclose)
internal/sendfile/sendfile_test.go:180:34: response body must be closed (bodyclose)
-internal/sendurl/sendurl.go:106: Function 'Inject' has too many statements (51 > 40) (funlen)
-internal/sendurl/sendurl.go:248:45: response body must be closed (bodyclose)
internal/testhelper/gitaly.go:277: 277-296 lines are duplicate of `internal/testhelper/gitaly.go:315-336` (dupl)
internal/testhelper/gitaly.go:315: 315-336 lines are duplicate of `internal/testhelper/gitaly.go:338-357` (dupl)
internal/testhelper/gitaly.go:338: 338-357 lines are duplicate of `internal/testhelper/gitaly.go:277-296` (dupl)
diff --git a/workhorse/internal/senddata/contentprocessor/contentprocessor.go b/workhorse/internal/senddata/contentprocessor/contentprocessor.go
index 48e8da28bc7d5df89cceed7aa7c5b605717b9833..4b68b9be01a29acbeb549144e0004a35a0038082 100644
--- a/workhorse/internal/senddata/contentprocessor/contentprocessor.go
+++ b/workhorse/internal/senddata/contentprocessor/contentprocessor.go
@@ -133,7 +133,7 @@ func (cd *contentDisposition) FlushError() error {
return err
}
- return http.NewResponseController(cd.rw).Flush()
+ return http.NewResponseController(cd.rw).Flush() //nolint:errcheck
}
// Unwrap lets http.ResponseController get the underlying http.ResponseWriter.
diff --git a/workhorse/internal/sendfile/sendfile_test.go b/workhorse/internal/sendfile/sendfile_test.go
index d1b5bfb79f7e511bd53ebedc5809038ccd64b449..6fe4ef9f36389bd7c62f71d0f4b2c2da27e2e890 100644
--- a/workhorse/internal/sendfile/sendfile_test.go
+++ b/workhorse/internal/sendfile/sendfile_test.go
@@ -177,7 +177,7 @@ func TestSendFileResponseWriterFlushable(t *testing.T) {
rw := httptest.NewRecorder()
sfrw := sendFileResponseWriter{rw: rw}
- rc := http.NewResponseController(&sfrw)
+ rc := http.NewResponseController(&sfrw) //nolint:errcheck
err := rc.Flush()
require.NoError(t, err, "the underlying response writer is not flushable")
diff --git a/workhorse/internal/sendurl/sendurl.go b/workhorse/internal/sendurl/sendurl.go
index e0c3cb326fa20515ab97303fad1b74a90ec20700..dda793a9176f637e7dfebdb15b7edbee36e11c61 100644
--- a/workhorse/internal/sendurl/sendurl.go
+++ b/workhorse/internal/sendurl/sendurl.go
@@ -113,9 +113,8 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
fail.Request(w, r, fmt.Errorf("SendURL: unpack sendData: %v", err))
return
}
- if params.Method == "" {
- params.Method = http.MethodGet
- }
+
+ setDefaultMethod(¶ms)
log.WithContextFields(r.Context(), log.Fields{
"url": mask.URL(params.URL),
@@ -128,12 +127,46 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
return
}
- // create new request and copy range headers
+ newReq, err := e.createNewRequest(w, r, ¶ms)
+ if err != nil {
+ return // Error handling is done in createNewRequest
+ }
+
+ resp, err := cachedClient(params).Do(newReq) //nolint:errcheck
+ if err != nil {
+ e.handleRequestError(w, r, err, ¶ms)
+ return
+ }
+ e.copyResponseHeaders(w, resp, params.ResponseHeaders)
+ w.WriteHeader(resp.StatusCode)
+
+ defer func() {
+ if err = resp.Body.Close(); err != nil {
+ fmt.Printf("Error closing response body: %v\n", err)
+ }
+ }()
+
+ if err := e.streamResponse(w, resp.Body); err != nil {
+ sendURLRequestsRequestFailed.Inc()
+ log.WithRequest(r).WithError(fmt.Errorf("SendURL: Copy response: %v", err)).Error()
+ return
+ }
+
+ sendURLRequestsSucceeded.Inc()
+}
+
+func setDefaultMethod(params *entryParams) {
+ if params.Method == "" {
+ params.Method = http.MethodGet
+ }
+}
+
+func (e *entry) createNewRequest(w http.ResponseWriter, r *http.Request, params *entryParams) (*http.Request, error) {
newReq, err := http.NewRequest(params.Method, params.URL, strings.NewReader(params.Body))
if err != nil {
sendURLRequestsInvalidData.Inc()
fail.Request(w, r, fmt.Errorf("SendURL: NewRequest: %v", err))
- return
+ return nil, err
}
newReq = newReq.WithContext(r.Context())
@@ -147,61 +180,43 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
}
}
- // execute new request
- resp, err := cachedClient(params).Do(newReq)
- if err != nil {
- status := http.StatusInternalServerError
+ return newReq, nil
+}
- if params.TimeoutResponseStatus != 0 && os.IsTimeout(err) {
- status = params.TimeoutResponseStatus
- } else if params.ErrorResponseStatus != 0 {
- status = params.ErrorResponseStatus
- }
+func (e *entry) handleRequestError(w http.ResponseWriter, r *http.Request, err error, params *entryParams) {
+ status := http.StatusInternalServerError
- sendURLRequestsRequestFailed.Inc()
- fail.Request(w, r, fmt.Errorf("SendURL: Do request: %v", err), fail.WithStatus(status))
- return
+ if params.TimeoutResponseStatus != 0 && os.IsTimeout(err) {
+ status = params.TimeoutResponseStatus
+ } else if params.ErrorResponseStatus != 0 {
+ status = params.ErrorResponseStatus
}
- // Prevent Go from adding a Content-Length header automatically
+ sendURLRequestsRequestFailed.Inc()
+ fail.Request(w, r, fmt.Errorf("SendURL: Do request: %v", err), fail.WithStatus(status))
+}
+
+func (e *entry) copyResponseHeaders(w http.ResponseWriter, resp *http.Response, responseHeaders map[string][]string) {
w.Header().Del("Content-Length")
- // copy response headers and body, except the headers from preserveHeaderKeys
for key, value := range resp.Header {
if !preserveHeaderKeys[key] {
w.Header()[key] = value
}
}
- // set response headers sent by rails
- for key, values := range params.ResponseHeaders {
+ for key, values := range responseHeaders {
w.Header().Del(key)
for _, value := range values {
w.Header().Add(key, value)
}
}
+}
- w.WriteHeader(resp.StatusCode)
-
- defer func() {
- if err = resp.Body.Close(); err != nil {
- fmt.Printf("Error closing response body: %v\n", err)
- }
- }()
-
- // Flushes the response right after it received.
- // Important for streaming responses, where content delivered in chunks.
- // Without flushing the body gets buffered by the HTTP server's internal buffer.
- n, err := io.Copy(newFlushingResponseWriter(w), resp.Body)
+func (e *entry) streamResponse(w http.ResponseWriter, body io.Reader) error {
+ n, err := io.Copy(newFlushingResponseWriter(w), body)
sendURLBytes.Add(float64(n))
-
- if err != nil {
- sendURLRequestsRequestFailed.Inc()
- log.WithRequest(r).WithError(fmt.Errorf("SendURL: Copy response: %v", err)).Error()
- return
- }
-
- sendURLRequestsSucceeded.Inc()
+ return err
}
func cachedClient(params entryParams) *http.Client {
@@ -245,7 +260,7 @@ func cachedClient(params entryParams) *http.Client {
func newFlushingResponseWriter(w http.ResponseWriter) *httpFlushingResponseWriter {
return &httpFlushingResponseWriter{
ResponseWriter: w,
- controller: http.NewResponseController(w),
+ controller: http.NewResponseController(w), //nolint:bodyclose
}
}