diff --git a/README.md b/README.md index d7c89c0..92f60dd 100644 --- a/README.md +++ b/README.md @@ -87,12 +87,16 @@ Usage of gcsproxy: Value for the Access-Control-Allow-Origin header. -i string Default index file to serve. + -log-errors + Log proxy error details at error level. -log-format string Log output format: text or json. (default "json") -log-level string Minimum log level: debug, info, warn, or error. (default "info") -not-found string Object served with HTTP 404 for unmatched routes. + -redact-errors + Suppress error response bodies. -spa SPA fallback: serve -i from the bucket root with HTTP 200 for unmatched routes. -v Show access log. diff --git a/main.go b/main.go index e172701..62dbc71 100644 --- a/main.go +++ b/main.go @@ -30,6 +30,8 @@ type Server struct { notFoundPath string contentLength bool corsOrigin string + redactErrors bool + logErrors bool verbose bool } @@ -47,6 +49,8 @@ func main() { logLevel = flag.String("log-level", "info", "Minimum log level: debug, info, warn, or error.") contentLength = flag.Bool("content-length", false, "Send the Content-Length header (disables chunked transfer).") corsOrigin = flag.String("cors-origin", "", "Value for the Access-Control-Allow-Origin header.") + redactErrors = flag.Bool("redact-errors", false, "Suppress error response bodies.") + logErrors = flag.Bool("log-errors", false, "Log proxy error details at error level.") ) flag.Parse() @@ -102,6 +106,8 @@ func main() { notFoundPath: *notFoundPath, contentLength: *contentLength, corsOrigin: *corsOrigin, + redactErrors: *redactErrors, + logErrors: *logErrors, verbose: *verbose, } @@ -173,7 +179,7 @@ func (s *Server) proxy(w http.ResponseWriter, r *http.Request, bucket, object st } } } - handleError(w, err) + s.handleError(w, err) return } if lastStrs, ok := r.Header["If-Modified-Since"]; ok && len(lastStrs) > 0 { @@ -193,7 +199,7 @@ func (s *Server) proxy(w http.ResponseWriter, r *http.Request, bucket, object st } if err := s.streamObject(w, r, attrs, http.StatusOK); err != nil { - handleError(w, err) + s.handleError(w, err) } } @@ -327,7 +333,7 @@ func (s *Server) streamRange(w http.ResponseWriter, r *http.Request, attrs *stor // (br, deflate, ...) are not transcoded and Range works normally. if strings.EqualFold(attrs.ContentEncoding, "gzip") { if err := s.streamObject(w, r, attrs, http.StatusOK); err != nil { - handleError(w, err) + s.handleError(w, err) } return } @@ -340,7 +346,7 @@ func (s *Server) streamRange(w http.ResponseWriter, r *http.Request, attrs *stor // that to unparseable byte-ranges so a malformed header doesn't // downgrade a previously-working download to 416. if err := s.streamObject(w, r, attrs, http.StatusOK); err != nil { - handleError(w, err) + s.handleError(w, err) } return case errors.Is(err, errRangeUnsatisfiable): @@ -369,7 +375,7 @@ func (s *Server) streamRange(w http.ResponseWriter, r *http.Request, attrs *stor objr, err := s.client.Bucket(attrs.Bucket).Object(attrs.Name).NewRangeReader(r.Context(), start, length) if err != nil { - handleError(w, err) + s.handleError(w, err) return } defer objr.Close() @@ -439,12 +445,19 @@ func healthCheck(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "OK\n") } -func handleError(w http.ResponseWriter, err error) { +func (s *Server) handleError(w http.ResponseWriter, err error) { + status := http.StatusInternalServerError if errors.Is(err, storage.ErrObjectNotExist) { - http.Error(w, err.Error(), http.StatusNotFound) - } else { - http.Error(w, err.Error(), http.StatusInternalServerError) + status = http.StatusNotFound + } + if s.logErrors { + slog.Error("proxy error", "status", status, "err", err) + } + if s.redactErrors { + w.WriteHeader(status) + return } + http.Error(w, err.Error(), status) } func header(r *http.Request, key string) (string, bool) { diff --git a/main_test.go b/main_test.go index fd3cc6a..2e454ad 100644 --- a/main_test.go +++ b/main_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "cloud.google.com/go/storage" "github.com/fsouza/fake-gcs-server/fakestorage" ) @@ -782,6 +783,115 @@ func TestProxy_NotFound_MissingPageFallsBackToDefault404(t *testing.T) { } } +// --- error redaction tests --- + +func TestProxy_RedactErrors_EmptyBodyOn404(t *testing.T) { + s := newTestServer(t, nil) + s.redactErrors = true + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/"+testBucket+"/missing.txt", nil) + s.handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + if got := rec.Body.String(); got != "" { + t.Errorf("body should be empty with -redact-errors, got %q", got) + } +} + +func TestProxy_RedactErrors_DisabledKeepsErrorBody(t *testing.T) { + s := newTestServer(t, nil) + // s.redactErrors left as false (zero value) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/"+testBucket+"/missing.txt", nil) + s.handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + if got := rec.Body.String(); !strings.Contains(got, storage.ErrObjectNotExist.Error()) { + t.Errorf("body = %q, want to contain %q", got, storage.ErrObjectNotExist.Error()) + } +} + +func TestProxy_RedactErrors_CORSHeaderStillSet(t *testing.T) { + s := newTestServer(t, nil) + s.redactErrors = true + s.corsOrigin = "*" + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/"+testBucket+"/missing.txt", nil) + s.handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { + t.Errorf("Access-Control-Allow-Origin = %q, want %q", got, "*") + } +} + +func TestProxy_ErrorIsLogged(t *testing.T) { + // With -log-errors, the full error detail is logged even when the + // response body is redacted. + s := newTestServer(t, nil) + s.redactErrors = true + s.logErrors = true + + var buf bytes.Buffer + installLogger(t, slog.NewJSONHandler(&buf, nil)) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/"+testBucket+"/missing.txt", nil) + s.handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + logLine := strings.TrimSpace(buf.String()) + if logLine == "" { + t.Fatal("no log output captured") + } + var entry map[string]any + if err := json.Unmarshal([]byte(logLine), &entry); err != nil { + t.Fatalf("log output is not valid JSON: %v\nraw: %s", err, logLine) + } + if got := entry["msg"]; got != "proxy error" { + t.Errorf("msg = %v, want %q", got, "proxy error") + } + if got := entry["level"]; got != "ERROR" { + t.Errorf("level = %v, want %q", got, "ERROR") + } + if got := entry["status"]; got != float64(http.StatusNotFound) { // json numbers are float64 + t.Errorf("status = %v, want %d", got, http.StatusNotFound) + } + if got, ok := entry["err"].(string); !ok || !strings.Contains(got, storage.ErrObjectNotExist.Error()) { + t.Errorf("err = %v, want to contain %q", entry["err"], storage.ErrObjectNotExist.Error()) + } +} + +func TestProxy_ErrorNotLoggedWithoutFlag(t *testing.T) { + s := newTestServer(t, nil) + // s.logErrors left as false (zero value) + + var buf bytes.Buffer + installLogger(t, slog.NewJSONHandler(&buf, nil)) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/"+testBucket+"/missing.txt", nil) + s.handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Errorf("unexpected log output without -log-errors: %q", got) + } +} + // --- structured logging tests --- // installLogger redirects slog.Default() to the given handler for the test