From f2a48d2ff3b6efab4148389d291072250c2628b8 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Tue, 23 Aug 2016 21:08:23 -0700 Subject: [PATCH] Fix AuthZ plugins headers change issue This fix tries to address the issue raised in 25927 where the HTTP headers have been chaged when AUthZ plugin is in place. This issue is that in `FlushAll` (`pkg/authorization/response.go`), the headers have been written (with `WriteHeader`) before all the headers have bee copied. This fix fixes the issue by placing `WriteHeader` after. A test has been added to cover the changes.` This fix fixes 25927 Signed-off-by: Yong Tang (cherry picked from commit 9cb8fb6ea03fcd78010ce7dd33585d96cd73e38c) Signed-off-by: Victor Vieux --- integration-cli/docker_cli_authz_unix_test.go | 19 +++++++++++++++++++ pkg/authorization/response.go | 12 +++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/integration-cli/docker_cli_authz_unix_test.go b/integration-cli/docker_cli_authz_unix_test.go index 427319a89e..a78fac0619 100644 --- a/integration-cli/docker_cli_authz_unix_test.go +++ b/integration-cli/docker_cli_authz_unix_test.go @@ -443,6 +443,25 @@ func (s *DockerAuthzSuite) TestAuthZPluginEnsureLoadImportWorking(c *check.C) { c.Assert(err, check.IsNil, check.Commentf(out)) } +func (s *DockerAuthzSuite) TestAuthZPluginHeader(c *check.C) { + c.Assert(s.d.Start("--debug", "--authorization-plugin="+testAuthZPlugin), check.IsNil) + s.ctrl.reqRes.Allow = true + s.ctrl.resRes.Allow = true + c.Assert(s.d.LoadBusybox(), check.IsNil) + + daemonURL, err := url.Parse(s.d.sock()) + + conn, err := net.DialTimeout(daemonURL.Scheme, daemonURL.Path, time.Second*10) + c.Assert(err, check.IsNil) + client := httputil.NewClientConn(conn, nil) + req, err := http.NewRequest("GET", "/version", nil) + c.Assert(err, check.IsNil) + resp, err := client.Do(req) + + c.Assert(err, check.IsNil) + c.Assert(resp.Header["Content-Type"][0], checker.Equals, "application/json") +} + // assertURIRecorded verifies that the given URI was sent and recorded in the authz plugin func assertURIRecorded(c *check.C, uris []string, uri string) { var found bool diff --git a/pkg/authorization/response.go b/pkg/authorization/response.go index 9eec82d42e..f7ce7364ce 100644 --- a/pkg/authorization/response.go +++ b/pkg/authorization/response.go @@ -175,11 +175,6 @@ func (rm *responseModifier) Flush() { // FlushAll flushes all data to the HTTP response func (rm *responseModifier) FlushAll() error { - // Copy the status code - if rm.statusCode > 0 { - rm.rw.WriteHeader(rm.statusCode) - } - // Copy the header for k, vv := range rm.header { for _, v := range vv { @@ -187,6 +182,13 @@ func (rm *responseModifier) FlushAll() error { } } + // Copy the status code + // Also WriteHeader needs to be done after all the headers + // have been copied (above). + if rm.statusCode > 0 { + rm.rw.WriteHeader(rm.statusCode) + } + var err error if len(rm.body) > 0 { // Write body