diff --git a/orbit/changes/33019-html-in-orbit-logs b/orbit/changes/33019-html-in-orbit-logs new file mode 100644 index 0000000000..e124f6e2da --- /dev/null +++ b/orbit/changes/33019-html-in-orbit-logs @@ -0,0 +1 @@ +* Improved orbit debug logs when response contains a large HTML page. diff --git a/server/service/base_client.go b/server/service/base_client.go index 2fb5436155..354ebb4216 100644 --- a/server/service/base_client.go +++ b/server/service/base_client.go @@ -81,7 +81,13 @@ func (bc *baseClient) parseResponse(verb, path string, response *http.Response, return fmt.Errorf("reading response body: %w", err) } if err := json.Unmarshal(b, &responseDest); err != nil { - return fmt.Errorf("decode %s %s response: %w, body: %s", verb, path, err, b) + const maxBodyLen = 200 + truncatedBytes, isHTML := truncateAndDetectHTML(b, maxBodyLen) + + if isHTML { + return fmt.Errorf("decode %s %s response: %w, (server returned HTML instead of JSON), body: %s", verb, path, err, truncatedBytes) + } + return fmt.Errorf("decode %s %s response: %w, body: %s", verb, path, err, truncatedBytes) } if e, ok := responseDest.(fleet.Errorer); ok { if e.Error() != nil { diff --git a/server/service/base_client_errors.go b/server/service/base_client_errors.go index 433c7603c0..b78b06ff1e 100644 --- a/server/service/base_client_errors.go +++ b/server/service/base_client_errors.go @@ -1,11 +1,13 @@ package service import ( + "bytes" "database/sql" "encoding/json" "errors" "fmt" "io" + "strings" "github.com/fleetdm/fleet/v4/server/fleet" ) @@ -104,15 +106,55 @@ type serverError struct { } `json:"errors"` } +// truncateAndDetectHTML truncates a response body to a reasonable length and +// detects if it's HTML content. Returns the truncated body and whether it's HTML. +func truncateAndDetectHTML(body []byte, maxLen int) (truncated []byte, isHTML bool) { + if len(body) > maxLen { + // Use append which is more idiomatic and efficient + truncated = append([]byte(nil), body[:maxLen]...) + truncated = append(truncated, "..."...) + } else { + // For small bodies, we can return the slice directly since it will be + // converted to string soon anyway and won't hold a large underlying array + truncated = body + } + lowerPrefix := bytes.ToLower(truncated) + isHTML = bytes.Contains(lowerPrefix, []byte("403 Forbidden

403 Forbidden

You don't have permission to access this resource.

`, + expected: "server returned HTML instead of JSON response, body: 403 Forbidden

403 Forbidden

You don't have permission to access this resource.

", + }, + { + name: "HTML with uppercase tags", + body: `ErrorServer Error`, + expected: "server returned HTML instead of JSON response, body: ErrorServer Error", + }, + { + name: "long HTML gets truncated", + body: `Error Page` + strings.Repeat("A", 200) + ``, + expected: "server returned HTML instead of JSON response, body: Error Page" + strings.Repeat("A", 135) + "...", + }, + { + name: "plain text error", + body: "Connection refused", + expected: "Connection refused", + }, + { + name: "empty response", + body: "", + expected: "empty response body", + }, + { + name: "long plain text truncated", + body: strings.Repeat("a", 250), + expected: strings.Repeat("a", 200) + "...", + }, + { + name: "invalid JSON", + body: `{invalid json}`, + expected: "{invalid json}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := strings.NewReader(tt.body) + result := extractServerErrorText(reader) + assert.Equal(t, tt.expected, result) + }) + } +}