From 9cb8fb6ea03fcd78010ce7dd33585d96cd73e38c 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 --- 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 f29a5fa91e..129bf2f417 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