From 4a0492f3e178c8ab970234b27702c76fedffec5f Mon Sep 17 00:00:00 2001 From: Hayder <96513935+jadidbourbaki@users.noreply.github.com> Date: Thu, 11 Apr 2024 17:19:24 -0400 Subject: [PATCH] admin: Make `Etag` a header, not a trailer (#6208) * Making eTags a header not a trailer * Checked the write * Fixed typo * Corrected comment * Added sync Pool * Changed control flow of buffer reset / putting and changed error code * Switched from interface{} to any in bufferPool --- admin.go | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/admin.go b/admin.go index c6151d11415..13d24084dab 100644 --- a/admin.go +++ b/admin.go @@ -954,17 +954,28 @@ func makeEtag(path string, hash hash.Hash) string { return fmt.Sprintf(`"%s %x"`, path, hash.Sum(nil)) } +// This buffer pool is used to keep buffers for +// reading the config file during eTag header generation +var bufferPool = sync.Pool{ + New: func() any { + return new(bytes.Buffer) + }, +} + func handleConfig(w http.ResponseWriter, r *http.Request) error { switch r.Method { case http.MethodGet: w.Header().Set("Content-Type", "application/json") - // Set the ETag as a trailer header. - // The alternative is to write the config to a buffer, and - // then hash that. - w.Header().Set("Trailer", "ETag") - hash := etagHasher() - configWriter := io.MultiWriter(w, hash) + + // Read the config into a buffer instead of writing directly to + // the response writer, as we want to set the ETag as the header, + // not the trailer. + buf := bufferPool.Get().(*bytes.Buffer) + buf.Reset() + defer bufferPool.Put(buf) + + configWriter := io.MultiWriter(buf, hash) err := readConfig(r.URL.Path, configWriter) if err != nil { return APIError{HTTPStatus: http.StatusBadRequest, Err: err} @@ -973,6 +984,10 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error { // we could consider setting up a sync.Pool for the summed // hashes to reduce GC pressure. w.Header().Set("Etag", makeEtag(r.URL.Path, hash)) + _, err = w.Write(buf.Bytes()) + if err != nil { + return APIError{HTTPStatus: http.StatusInternalServerError, Err: err} + } return nil