mirror of
https://github.com/supabase/supabase.git
synced 2026-10-06 09:55:06 +03:00
fix(studio): duplicate Content-Type on webhook log drains (#46673)
## Problem Webhook log drains (project and org/audit) deliver requests with a malformed, duplicated `Content-Type: application/jsonapplication/json` header. On at least some receivers this breaks body parsing, so the delivered body appears empty even though it is present (confirmed with gzip both on and off). Root cause: the create form seeds a default `Content-Type: application/json` header for webhook drains, and the logflare webhook adaptor's `Tesla.Middleware.JSON` also sets `content-type: application/json` when it encodes the body. Both are sent, and the receiver concatenates the two same-named headers. ## Fix Stop seeding `Content-Type` in the webhook default headers (`getDefaultHeadersByType`). The delivery side already sets it, so a single clean header is sent. OTLP keeps its `application/x-protobuf` default because the OTLP delivery path uses `json: false` and does not set a content type itself. Updated the form tests that assumed the seeded header (the added-header row is now index 0 instead of 1, and the duplicate-header test now adds two explicit rows). ## How to test - Create a webhook (Custom Endpoint) audit log drain pointing at a request bin. - Trigger an audit event and inspect the delivered request: `Content-Type` should be a single `application/json`, and the JSON body should be visible. ## Note This fixes the common case (the seeded default). A user who manually adds a `Content-Type` header to a webhook drain would still hit the duplication; the robust cross-team fix would be for the logflare webhook adaptor to drop an incoming `content-type` before its JSON middleware sets one. Flagging for the logs team. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Log Drain header handling corrected: webhook drains no longer add a default Content-Type; other drain types retain their appropriate defaults. Empty header rows are no longer submitted. * **Tests** * Updated tests to match new header indexing, validation behavior, and submission expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
2cb7f0c078
commit
34163fc0ca
3 files changed
+18
-18
No files matched your search
@@ -85,13 +85,13 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
useTrackMock.mockReturnValue(trackMock)
|
||||
})
|
||||
|
||||
it('shows the JSON content type header for webhook create mode', async () => {
|
||||
it('does not prefill a Content-Type header for webhook create mode', async () => {
|
||||
renderForm()
|
||||
|
||||
await screen.findByRole('dialog')
|
||||
|
||||
expect(screen.getByDisplayValue('Content-Type')).toBeInTheDocument()
|
||||
expect(screen.getByDisplayValue('application/json')).toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('Content-Type')).not.toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('application/json')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('wraps the save button with the save destination shortcut while open', async () => {
|
||||
@@ -169,8 +169,8 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
const headerNameInputs = screen.getAllByPlaceholderText('Header name')
|
||||
const headerValueInputs = screen.getAllByPlaceholderText('Header value')
|
||||
|
||||
await user.type(headerNameInputs[1], 'X-API-Key')
|
||||
await user.type(headerValueInputs[1], 'secret-key')
|
||||
await user.type(headerNameInputs[0], 'X-API-Key')
|
||||
await user.type(headerValueInputs[0], 'secret-key')
|
||||
|
||||
submitForm()
|
||||
|
||||
@@ -179,7 +179,6 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
expect.objectContaining({
|
||||
type: 'webhook',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'X-API-Key': 'secret-key',
|
||||
},
|
||||
})
|
||||
@@ -199,12 +198,15 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
'https://logs.example.com/ingest'
|
||||
)
|
||||
await user.click(screen.getByRole('button', { name: 'Add a new header' }))
|
||||
await user.click(screen.getByRole('button', { name: 'Add a new header' }))
|
||||
|
||||
const headerNameInputs = screen.getAllByPlaceholderText('Header name')
|
||||
const headerValueInputs = screen.getAllByPlaceholderText('Header value')
|
||||
|
||||
await user.type(headerNameInputs[1], 'Content-Type')
|
||||
await user.type(headerValueInputs[1], 'application/custom')
|
||||
await user.type(headerNameInputs[0], 'X-Custom')
|
||||
await user.type(headerValueInputs[0], 'one')
|
||||
await user.type(headerNameInputs[1], 'X-Custom')
|
||||
await user.type(headerValueInputs[1], 'two')
|
||||
|
||||
submitForm()
|
||||
|
||||
@@ -231,11 +233,9 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
expect(onSubmit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: 'webhook',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
},
|
||||
})
|
||||
)
|
||||
expect(onSubmit.mock.calls[0][0]).not.toHaveProperty('headers')
|
||||
expect(screen.queryByText('undefined')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
@@ -253,7 +253,7 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
await user.click(screen.getByRole('button', { name: 'Add a new header' }))
|
||||
|
||||
const headerNameInputs = screen.getAllByPlaceholderText('Header name')
|
||||
await user.type(headerNameInputs[1], 'X-Draft-Only')
|
||||
await user.type(headerNameInputs[0], 'X-Draft-Only')
|
||||
|
||||
submitForm()
|
||||
|
||||
@@ -276,7 +276,7 @@ describe('LogDrainDestinationSheetForm', () => {
|
||||
await user.click(screen.getByRole('button', { name: 'Add a new header' }))
|
||||
|
||||
const headerValueInputs = screen.getAllByPlaceholderText('Header value')
|
||||
await user.type(headerValueInputs[1], 'draft-value')
|
||||
await user.type(headerValueInputs[0], 'draft-value')
|
||||
|
||||
submitForm()
|
||||
|
||||
|
||||
@@ -36,10 +36,8 @@ describe('getHeadersSectionDescription', () => {
|
||||
})
|
||||
|
||||
describe('getDefaultHeadersByType', () => {
|
||||
it('returns the JSON content type header for webhook destinations', () => {
|
||||
expect(getDefaultHeadersByType('webhook')).toEqual({
|
||||
'Content-Type': 'application/json',
|
||||
})
|
||||
it('does not return a default Content-Type header for webhook destinations', () => {
|
||||
expect(getDefaultHeadersByType('webhook')).toEqual({})
|
||||
})
|
||||
|
||||
it('returns the protobuf content type header for OTLP destinations', () => {
|
||||
|
||||
@@ -40,8 +40,10 @@ export const HEADER_VALIDATION_ERRORS = {
|
||||
VALUE_REQUIRED: 'Header value is required',
|
||||
} as const
|
||||
|
||||
// Webhook drains intentionally omit a Content-Type default: the delivery side already sets
|
||||
// `application/json`, and seeding it here produces a duplicated Content-Type header that can
|
||||
// break body parsing on the receiver. OTLP needs it since its delivery does not set one.
|
||||
const DEFAULT_HEADERS_BY_TYPE: Partial<Record<LogDrainType, Record<string, string>>> = {
|
||||
webhook: { 'Content-Type': 'application/json' },
|
||||
otlp: { 'Content-Type': 'application/x-protobuf' },
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user