From 928242a7383b99cfcd678d0f60c8771d12568b98 Mon Sep 17 00:00:00 2001 From: Noste <83548733+Noooste@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:16:38 +0200 Subject: [PATCH] fix(backend): handle gzip decompression for API responses --- backend/internal/services/admin_v1.go | 14 ++--- backend/internal/services/admin_v2.go | 41 ++++++++++---- backend/internal/services/admin_v2_test.go | 65 ++++++++++++++++++++++ 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/backend/internal/services/admin_v1.go b/backend/internal/services/admin_v1.go index 81f3900..cd2355b 100644 --- a/backend/internal/services/admin_v1.go +++ b/backend/internal/services/admin_v1.go @@ -3,11 +3,10 @@ package services import ( "Noooste/garage-ui/internal/config" "Noooste/garage-ui/internal/models" - "Noooste/garage-ui/pkg/utils" logpkg "Noooste/garage-ui/pkg/logger" + "Noooste/garage-ui/pkg/utils" "context" "fmt" - "io" "net/http" "time" @@ -363,13 +362,12 @@ func (s *GarageV1AdminService) GetMetrics(ctx context.Context) (string, error) { return "", fmt.Errorf("request failed: %w", err) } defer resp.RawBody.Close() - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - bodyBytes, _ := io.ReadAll(resp.RawBody) - return "", fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(bodyBytes)) - } - bodyBytes, err := io.ReadAll(resp.RawBody) + bodyBytes, err := readResponseBody(resp) if err != nil { - return "", fmt.Errorf("failed to read response: %w", err) + return "", err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "", fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(bodyBytes)) } return string(bodyBytes), nil } diff --git a/backend/internal/services/admin_v2.go b/backend/internal/services/admin_v2.go index bdf7ecd..d357704 100644 --- a/backend/internal/services/admin_v2.go +++ b/backend/internal/services/admin_v2.go @@ -3,12 +3,11 @@ package services import ( "Noooste/garage-ui/internal/config" "Noooste/garage-ui/internal/models" - "Noooste/garage-ui/pkg/utils" logpkg "Noooste/garage-ui/pkg/logger" + "Noooste/garage-ui/pkg/utils" "context" "encoding/json" "fmt" - "io" "net/http" "time" @@ -63,17 +62,38 @@ func (s *GarageV2AdminService) doRequest(ctx context.Context, method, path strin return resp, nil } -// decodeResponse decodes a JSON response into the target structure +// readResponseBody reads and decompresses the response body. +// +// doRequest sets IgnoreBody, so azuretls does not read or decompress the body +// for us. Over HTTP/1.1 the underlying transport transparently gunzips the body +// and clears Content-Encoding, but over HTTP/2 (e.g. Garage behind a reverse +// proxy) the body is delivered still-compressed with Content-Encoding set. We +// therefore decode according to that header before touching the payload; +// otherwise a gzip'd body reaches the JSON parser as raw bytes and fails with +// "invalid character '\x1f'" (0x1f is the gzip magic byte). See issue #95. +func readResponseBody(resp *azuretls.Response) ([]byte, error) { + bodyBytes, err := azuretls.DecodeResponseBody(resp.RawBody, resp.Header.Get("Content-Encoding")) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return bodyBytes, nil +} + +// decodeResponse decodes a JSON response into the target structure. func decodeResponse(resp *azuretls.Response, target interface{}) error { defer resp.RawBody.Close() + bodyBytes, err := readResponseBody(resp) + if err != nil { + return err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { - bodyBytes, _ := io.ReadAll(resp.RawBody) return fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(bodyBytes)) } if target != nil { - if err := json.NewDecoder(resp.RawBody).Decode(target); err != nil { + if err := json.Unmarshal(bodyBytes, target); err != nil { return fmt.Errorf("failed to decode response: %w", err) } } @@ -535,14 +555,13 @@ func (s *GarageV2AdminService) GetMetrics(ctx context.Context) (string, error) { } defer resp.RawBody.Close() - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - bodyBytes, _ := io.ReadAll(resp.RawBody) - return "", fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(bodyBytes)) + bodyBytes, err := readResponseBody(resp) + if err != nil { + return "", err } - bodyBytes, err := io.ReadAll(resp.RawBody) - if err != nil { - return "", fmt.Errorf("failed to read response: %w", err) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "", fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(bodyBytes)) } return string(bodyBytes), nil diff --git a/backend/internal/services/admin_v2_test.go b/backend/internal/services/admin_v2_test.go index 59763a3..4842d4c 100644 --- a/backend/internal/services/admin_v2_test.go +++ b/backend/internal/services/admin_v2_test.go @@ -1,6 +1,8 @@ package services import ( + "bytes" + "compress/gzip" "context" "encoding/json" "io" @@ -13,6 +15,9 @@ import ( "Noooste/garage-ui/internal/config" "Noooste/garage-ui/internal/models" + + "github.com/Noooste/azuretls-client" + fhttp "github.com/Noooste/fhttp" ) // newAdminTestServer wires an httptest.Server (with the supplied handler) to a @@ -531,6 +536,66 @@ func TestDoRequest_MalformedJSONReturnsDecodeError(t *testing.T) { } } +// gzipBytes gzip-compresses b, mirroring what a reverse proxy or the Garage +// admin API emits when Content-Encoding: gzip negotiation succeeds. +func gzipBytes(t *testing.T, b []byte) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + if _, err := gz.Write(b); err != nil { + t.Fatalf("gzip write: %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("gzip close: %v", err) + } + return buf.Bytes() +} + +// TestDecodeResponse_GzipEncodedBody reproduces issue #95. Over HTTP/2 (Garage +// 2.3.0 behind a reverse proxy) the transport does NOT transparently gunzip, so +// decodeResponse receives a still-compressed RawBody with Content-Encoding:gzip. +// Feeding that raw gzip stream to the JSON decoder failed with: +// "invalid character '\x1f' looking for beginning of value" (0x1f is the gzip +// magic byte). decodeResponse must honor Content-Encoding and decompress first. +func TestDecodeResponse_GzipEncodedBody(t *testing.T) { + want := &models.GarageBucketInfo{ID: "gz-bucket"} + payload, err := json.Marshal(want) + if err != nil { + t.Fatalf("marshal: %v", err) + } + resp := &azuretls.Response{ + StatusCode: http.StatusOK, + Header: fhttp.Header{"Content-Encoding": []string{"gzip"}}, + RawBody: io.NopCloser(bytes.NewReader(gzipBytes(t, payload))), + } + + var got models.GarageBucketInfo + if err := decodeResponse(resp, &got); err != nil { + t.Fatalf("decodeResponse with gzip body: %v", err) + } + if got.ID != want.ID { + t.Errorf("ID = %q, want %q", got.ID, want.ID) + } +} + +// TestDecodeResponse_GzipEncodedErrorBody ensures a compressed non-2xx body is +// also decompressed before being echoed into the error message. +func TestDecodeResponse_GzipEncodedErrorBody(t *testing.T) { + resp := &azuretls.Response{ + StatusCode: http.StatusInternalServerError, + Header: fhttp.Header{"Content-Encoding": []string{"gzip"}}, + RawBody: io.NopCloser(bytes.NewReader(gzipBytes(t, []byte("boom")))), + } + + err := decodeResponse(resp, nil) + if err == nil { + t.Fatal("expected error for 500 response, got nil") + } + if !strings.Contains(err.Error(), "boom") { + t.Errorf("error %q should contain decompressed body %q", err.Error(), "boom") + } +} + // TestAllMethods_Non2xxReturnsError exercises the decodeResponse error branch // of every admin method by pointing them at a server that always returns 500. // This is a single sweep over the near-identical "if err := decodeResponse ...