Skip to content

Commit

Permalink
fix: default to json if client accepts all media types (#6950)
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig authored Jul 15, 2024
1 parent 1a35ee8 commit 9103f56
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
7 changes: 6 additions & 1 deletion packages/api/src/utils/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ export function parseAcceptHeader(accept?: string, supported = SUPPORTED_MEDIA_T
// Normalize here, using 1 as the default qvalue
const quality = current.includes(";") ? current.split(";") : [current, "q=1"];

const mediaType = quality[0].trim();
let mediaType = quality[0].trim();

if (mediaType === "*/*") {
// Default to json if all media types are accepted
mediaType = MediaType.json;
}

// If the mime type isn't acceptable, move on to the next entry
if (!isSupportedMediaType(mediaType, supported)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/utils/server/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function createFastifyHandler<E extends Endpoint>(
if (definition.resp.isEmpty) {
// Ignore Accept header, the response will be sent without body
responseMediaType = null;
} else if (acceptHeader === undefined || acceptHeader === "*/*") {
} else if (acceptHeader === undefined) {
// Default to json to not force user to set header, e.g. when using curl
responseMediaType = MediaType.json;
} else {
Expand Down
10 changes: 9 additions & 1 deletion packages/api/test/unit/utils/headers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ describe("utils / headers", () => {
describe("parseAcceptHeader", () => {
const testCases: {header: string | undefined; expected: MediaType | null}[] = [
{header: undefined, expected: null},
{header: "*/*", expected: null},
{header: "*/*", expected: MediaType.json},
{header: "application/json", expected: MediaType.json},
{header: "application/octet-stream", expected: MediaType.ssz},
{header: "application/invalid", expected: null},
{header: "application/invalid;q=1,application/octet-stream;q=0.1", expected: MediaType.ssz},
{header: "application/octet-stream;q=0.5,application/json;q=1", expected: MediaType.json},
{header: "application/octet-stream;q=1,application/json;q=0.1", expected: MediaType.ssz},
{header: "application/octet-stream;q=1,application/json;q=0.9", expected: MediaType.ssz},
{header: "application/octet-stream;q=1,*/*;q=0.9", expected: MediaType.ssz},
{header: "application/octet-stream,application/json;q=0.1", expected: MediaType.ssz},
{header: "application/octet-stream;,application/json;q=0.1", expected: MediaType.json},
{header: "application/octet-stream;q=2,application/json;q=0.1", expected: MediaType.json},
Expand All @@ -20,6 +22,12 @@ describe("utils / headers", () => {
{header: "application/octet-stream ; q=0.5 , application/json ; q=1", expected: MediaType.json},
{header: "application/octet-stream ; q=1 , application/json ; q=0.1", expected: MediaType.ssz},
{header: "application/octet-stream;q=1,application/json;q=0.1", expected: MediaType.ssz},
{
// Default Accept header set by chrome browser
header:
"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7",
expected: MediaType.json,
},

// The implementation is order dependent, however, RFC-9110 doesn't specify a preference.
// The following tests serve to document the behavior at the time of implementation- not a
Expand Down

0 comments on commit 9103f56

Please sign in to comment.