Skip to content

Commit

Permalink
Restore previous behaviour of no trailing newlines in HTTP error resp…
Browse files Browse the repository at this point in the history
…onse bodies (#11606)

**What this PR does / why we need it**:
#11487 introduced a regression which added trailing newlines to HTTP
error response bodies, which may affect client integrations.

This regression hasn't made it into the 2.9.x or 2.8.x releases yet so
no backporting is necessary.
  • Loading branch information
Danny Kopping authored Jan 8, 2024
1 parent 142d356 commit 627e093
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
* [11195](https://github.com/grafana/loki/pull/11195) **canuteson** Generate tsdb_shipper storage_config even if using_boltdb_shipper is false
* [11551](https://github.com/grafana/loki/pull/11551) **dannykopping** Do not reflect label names in request metrics' "route" label.
* [11601](https://github.com/grafana/loki/pull/11601) **dannykopping** Ruler: Fixed a panic that can be caused by concurrent read-write access of tenant configs when there are a large amount of rules.
* [11606](https://github.com/grafana/loki/pull/11606) **dannykopping** Fixed regression adding newlines to HTTP error response bodies which may break client integrations.

##### Changes

Expand Down
2 changes: 1 addition & 1 deletion pkg/querier/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestTailHandler(t *testing.T) {

handler.ServeHTTP(rr, req)
require.Equal(t, http.StatusBadRequest, rr.Code)
require.Equal(t, "multiple org IDs present\n", rr.Body.String())
require.Equal(t, "multiple org IDs present", rr.Body.String())
}

type slowConnectionSimulator struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/querier/queryrange/serialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestResponseFormat(t *testing.T) {
url: "/loki/wrong/path",
response: nil,
expectedCode: http.StatusNotFound,
expectedRespone: "unknown request path: /loki/wrong/path\n",
expectedRespone: "unknown request path: /loki/wrong/path",
},
} {
t.Run(fmt.Sprintf("%s returns the expected format", tc.url), func(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/util/server/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"context"
"errors"
"fmt"
"net/http"

"github.com/grafana/dskit/httpgrpc"
Expand All @@ -29,7 +30,10 @@ const (
// WriteError write a go error with the correct status code.
func WriteError(err error, w http.ResponseWriter) {
status, cerr := ClientHTTPStatusAndError(err)
http.Error(w, cerr.Error(), status)
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(status)
fmt.Fprint(w, cerr.Error())
}

// ClientHTTPStatusAndError returns error and http status that is "safe" to return to client without
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/server/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func Test_writeError(t *testing.T) {
require.Equal(t, tt.expectedStatus, rec.Result().StatusCode)
b, err := io.ReadAll(rec.Result().Body)
require.NoError(t, err)
require.Equal(t, tt.msg, string(b[:len(b)-1]))
require.EqualValues(t, tt.msg, b)
})

t.Run(tt.name+"-roundtrip", func(t *testing.T) {
Expand All @@ -68,7 +68,7 @@ func Test_writeError(t *testing.T) {
require.Equal(t, tt.expectedStatus, rec.Result().StatusCode)
b, err := io.ReadAll(rec.Result().Body)
require.NoError(t, err)
require.Equal(t, tt.msg, string(b[:len(b)-1]))
require.EqualValues(t, tt.msg, b)
})
}
}

0 comments on commit 627e093

Please sign in to comment.