Commit 5e972b72 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Disable NGINX response buffering when not proxying

parent 02eb4ffe
...@@ -10,6 +10,8 @@ import ( ...@@ -10,6 +10,8 @@ import (
"syscall" "syscall"
) )
const NginxResponseBufferHeader = "X-Accel-Buffering"
func Fail500(w http.ResponseWriter, r *http.Request, err error) { func Fail500(w http.ResponseWriter, r *http.Request, err error) {
http.Error(w, "Internal server error", 500) http.Error(w, "Internal server error", 500)
captureRavenError(r, err) captureRavenError(r, err)
...@@ -132,3 +134,11 @@ func ExitStatus(err error) (int, bool) { ...@@ -132,3 +134,11 @@ func ExitStatus(err error) (int, bool) {
return waitStatus.ExitStatus(), true return waitStatus.ExitStatus(), true
} }
func DisableResponseBuffering(w http.ResponseWriter) {
w.Header().Set(NginxResponseBufferHeader, "no")
}
func AllowResponseBuffering(w http.ResponseWriter) {
w.Header().Del(NginxResponseBufferHeader)
}
...@@ -34,5 +34,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { ...@@ -34,5 +34,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
req.Header.Set("Gitlab-Workhorse", p.Version) req.Header.Set("Gitlab-Workhorse", p.Version)
req.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano())) req.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano()))
helper.AllowResponseBuffering(w)
p.reverseProxy.ServeHTTP(w, &req) p.reverseProxy.ServeHTTP(w, &req)
} }
...@@ -2,6 +2,8 @@ package senddata ...@@ -2,6 +2,8 @@ package senddata
import ( import (
"net/http" "net/http"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
) )
type sendDataResponseWriter struct { type sendDataResponseWriter struct {
...@@ -62,6 +64,7 @@ func (s *sendDataResponseWriter) tryInject() bool { ...@@ -62,6 +64,7 @@ func (s *sendDataResponseWriter) tryInject() bool {
for _, injecter := range s.injecters { for _, injecter := range s.injecters {
if injecter.Match(header) { if injecter.Match(header) {
s.hijacked = true s.hijacked = true
helper.DisableResponseBuffering(s.rw)
injecter.Inject(s.rw, s.req, header) injecter.Inject(s.rw, s.req, header)
return true return true
} }
......
...@@ -66,6 +66,7 @@ func (s *sendFileResponseWriter) WriteHeader(status int) { ...@@ -66,6 +66,7 @@ func (s *sendFileResponseWriter) WriteHeader(status int) {
s.hijacked = true s.hijacked = true
// Serve the file // Serve the file
helper.DisableResponseBuffering(s.rw)
sendFileFromDisk(s.rw, s.req, file) sendFileFromDisk(s.rw, s.req, file)
return return
} }
......
...@@ -65,6 +65,8 @@ func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) { ...@@ -65,6 +65,8 @@ func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) {
w := helper.NewLoggingResponseWriter(ow) w := helper.NewLoggingResponseWriter(ow)
defer w.Log(r) defer w.Log(r)
helper.DisableResponseBuffering(&w)
// Drop WebSocket connection and CONNECT method // Drop WebSocket connection and CONNECT method
if r.RequestURI == "*" { if r.RequestURI == "*" {
helper.HTTPError(&w, r, "Connection upgrade not allowed", http.StatusBadRequest) helper.HTTPError(&w, r, "Connection upgrade not allowed", http.StatusBadRequest)
......
...@@ -362,6 +362,9 @@ func TestRegularProjectsAPI(t *testing.T) { ...@@ -362,6 +362,9 @@ func TestRegularProjectsAPI(t *testing.T) {
if resp.StatusCode != 200 { if resp.StatusCode != 200 {
t.Errorf("GET %q: expected 200, got %d", resource, resp.StatusCode) t.Errorf("GET %q: expected 200, got %d", resource, resp.StatusCode)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "" {
t.Errorf("GET %q: Expected %s not to be present, got %q", resource, helper.NginxResponseBufferHeader, h)
}
} }
} }
...@@ -416,6 +419,9 @@ func TestAllowedStaticFile(t *testing.T) { ...@@ -416,6 +419,9 @@ func TestAllowedStaticFile(t *testing.T) {
if proxied { if proxied {
t.Errorf("GET %q: should not have made it to backend", resource) t.Errorf("GET %q: should not have made it to backend", resource)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("GET %q: Expected %s to equal %q, got %q", resource, helper.NginxResponseBufferHeader, "no", h)
}
} }
} }
...@@ -488,6 +494,9 @@ func TestAllowedPublicUploadsFile(t *testing.T) { ...@@ -488,6 +494,9 @@ func TestAllowedPublicUploadsFile(t *testing.T) {
if !proxied { if !proxied {
t.Fatalf("GET %q: never made it to backend", resource) t.Fatalf("GET %q: never made it to backend", resource)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("GET %q: Expected %s to equal %q, got %q", resource, helper.NginxResponseBufferHeader, "no", h)
}
} }
} }
...@@ -642,6 +651,10 @@ func TestArtifactsGetSingleFile(t *testing.T) { ...@@ -642,6 +651,10 @@ func TestArtifactsGetSingleFile(t *testing.T) {
if string(body) != fileContents { if string(body) != fileContents {
t.Fatalf("Expected file contents %q, got %q", fileContents, body) t.Fatalf("Expected file contents %q, got %q", fileContents, body)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("GET %q: Expected %s to equal %q, got %q", resourcePath, helper.NginxResponseBufferHeader, "no", h)
}
} }
func TestGetGitBlob(t *testing.T) { func TestGetGitBlob(t *testing.T) {
...@@ -669,6 +682,10 @@ func TestGetGitBlob(t *testing.T) { ...@@ -669,6 +682,10 @@ func TestGetGitBlob(t *testing.T) {
if !strings.HasPrefix(string(body), "The MIT License (MIT)") { if !strings.HasPrefix(string(body), "The MIT License (MIT)") {
t.Fatalf("Expected MIT license, got %q", body) t.Fatalf("Expected MIT license, got %q", body)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("Expected %s to equal %q, got %q", helper.NginxResponseBufferHeader, "no", h)
}
} }
func TestGetGitDiff(t *testing.T) { func TestGetGitDiff(t *testing.T) {
...@@ -693,6 +710,10 @@ func TestGetGitDiff(t *testing.T) { ...@@ -693,6 +710,10 @@ func TestGetGitDiff(t *testing.T) {
if bodyLengthBytes != 155 { if bodyLengthBytes != 155 {
t.Fatal("Expected the body to consist of 155 bytes, got %v", bodyLengthBytes) t.Fatal("Expected the body to consist of 155 bytes, got %v", bodyLengthBytes)
} }
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("Expected %s to equal %q, got %q", helper.NginxResponseBufferHeader, "no", h)
}
} }
func TestGetGitPatch(t *testing.T) { func TestGetGitPatch(t *testing.T) {
...@@ -712,6 +733,10 @@ func TestGetGitPatch(t *testing.T) { ...@@ -712,6 +733,10 @@ func TestGetGitPatch(t *testing.T) {
// Only the two commits on the fix branch should be included // Only the two commits on the fix branch should be included
testhelper.AssertPatchSeries(t, body, "12d65c8dd2b2676fa3ac47d955accc085a37a9c1", toSha) testhelper.AssertPatchSeries(t, body, "12d65c8dd2b2676fa3ac47d955accc085a37a9c1", toSha)
if h := resp.Header.Get(helper.NginxResponseBufferHeader); h != "no" {
t.Errorf("Expected %s to equal %q, got %q", helper.NginxResponseBufferHeader, "no", h)
}
} }
func TestApiContentTypeBlock(t *testing.T) { func TestApiContentTypeBlock(t *testing.T) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment