Skip to content

Commit

Permalink
feat: support qvalue weighting in Accept headers (#5966)
Browse files Browse the repository at this point in the history
  • Loading branch information
jshufro committed Oct 2, 2023
1 parent b46dfcb commit 299406f
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 18 deletions.
8 changes: 4 additions & 4 deletions packages/api/src/beacon/client/debug.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {ChainForkConfig} from "@lodestar/config";
import {ApiClientResponse} from "../../interfaces.js";
import {ApiClientResponse, ResponseFormat} from "../../interfaces.js";
import {HttpStatusCode} from "../../utils/client/httpStatusCode.js";
import {generateGenericJsonClient, getFetchOptsSerializers, IHttpClient} from "../../utils/client/index.js";
import {StateId} from "../routes/beacon/state.js";
import {Api, getReqSerializers, getReturnTypes, ReqTypes, routesData, StateFormat} from "../routes/debug.js";
import {Api, getReqSerializers, getReturnTypes, ReqTypes, routesData} from "../routes/debug.js";

// As Jul 2022, it takes up to 3 mins to download states so make this 5 mins for reservation
const GET_STATE_TIMEOUT_MS = 5 * 60 * 1000;
Expand All @@ -25,7 +25,7 @@ export function getClient(_config: ChainForkConfig, httpClient: IHttpClient): Ap
// TODO: Debug the type issue
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
async getState(stateId: string, format?: StateFormat) {
async getState(stateId: string, format?: ResponseFormat) {
if (format === "ssz") {
const res = await httpClient.arrayBuffer({
...fetchOptsSerializers.getState(stateId, format),
Expand All @@ -43,7 +43,7 @@ export function getClient(_config: ChainForkConfig, httpClient: IHttpClient): Ap
// TODO: Debug the type issue
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
async getStateV2(stateId: StateId, format?: StateFormat) {
async getStateV2(stateId: StateId, format?: ResponseFormat) {
if (format === "ssz") {
const res = await httpClient.arrayBuffer({
...fetchOptsSerializers.getStateV2(stateId, format),
Expand Down
6 changes: 3 additions & 3 deletions packages/api/src/beacon/routes/beacon/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ContainerData,
} from "../../../utils/index.js";
import {HttpStatusCode} from "../../../utils/client/httpStatusCode.js";
import {parseAcceptHeader, writeAcceptHeader} from "../../../utils/acceptHeader.js";
import {ApiClientResponse, ResponseFormat} from "../../../interfaces.js";
import {
SignedBlockContents,
Expand All @@ -31,7 +32,6 @@ import {
// See /packages/api/src/routes/index.ts for reasoning and instructions to add new routes

export type BlockId = RootHex | Slot | "head" | "genesis" | "finalized";
export const mimeTypeSSZ = "application/octet-stream";

/**
* True if the response references an unverified execution payload. Optimistic information may be invalidated at
Expand Down Expand Up @@ -283,9 +283,9 @@ export function getReqSerializers(config: ChainForkConfig): ReqSerializers<Api,
const getBlockReq: ReqSerializer<Api["getBlock"], GetBlockReq> = {
writeReq: (block_id, format) => ({
params: {block_id: String(block_id)},
headers: {accept: format === "ssz" ? mimeTypeSSZ : "application/json"},
headers: {accept: writeAcceptHeader(format)},
}),
parseReq: ({params, headers}) => [params.block_id, headers.accept === mimeTypeSSZ ? "ssz" : "json"],
parseReq: ({params, headers}) => [params.block_id, parseAcceptHeader(headers.accept)],
schema: {params: {block_id: Schema.StringRequired}},
};

Expand Down
14 changes: 6 additions & 8 deletions packages/api/src/beacon/routes/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ import {
ContainerData,
} from "../../utils/index.js";
import {HttpStatusCode} from "../../utils/client/httpStatusCode.js";
import {ApiClientResponse} from "../../interfaces.js";
import {parseAcceptHeader, writeAcceptHeader} from "../../utils/acceptHeader.js";
import {ApiClientResponse, ResponseFormat} from "../../interfaces.js";
import {ExecutionOptimistic, StateId} from "./beacon/state.js";

// See /packages/api/src/routes/index.ts for reasoning and instructions to add new routes

export type StateFormat = "json" | "ssz";
export const mimeTypeSSZ = "application/octet-stream";

const stringType = new StringType();
const protoNodeSszType = new ContainerType(
{
Expand Down Expand Up @@ -91,7 +89,7 @@ export type Api = {
getState(stateId: StateId, format: "ssz"): Promise<ApiClientResponse<{[HttpStatusCode.OK]: Uint8Array}>>;
getState(
stateId: StateId,
format?: StateFormat
format?: ResponseFormat
): Promise<
ApiClientResponse<{
[HttpStatusCode.OK]: Uint8Array | {data: allForks.BeaconState; executionOptimistic: ExecutionOptimistic};
Expand All @@ -117,7 +115,7 @@ export type Api = {
getStateV2(stateId: StateId, format: "ssz"): Promise<ApiClientResponse<{[HttpStatusCode.OK]: Uint8Array}>>;
getStateV2(
stateId: StateId,
format?: StateFormat
format?: ResponseFormat
): Promise<
ApiClientResponse<{
[HttpStatusCode.OK]:
Expand Down Expand Up @@ -149,9 +147,9 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
const getState: ReqSerializer<Api["getState"], ReqTypes["getState"]> = {
writeReq: (state_id, format) => ({
params: {state_id: String(state_id)},
headers: {accept: format === "ssz" ? mimeTypeSSZ : "application/json"},
headers: {accept: writeAcceptHeader(format)},
}),
parseReq: ({params, headers}) => [params.state_id, headers.accept === mimeTypeSSZ ? "ssz" : "json"],
parseReq: ({params, headers}) => [params.state_id, parseAcceptHeader(headers.accept)],
schema: {params: {state_id: Schema.StringRequired}},
};

Expand Down
81 changes: 81 additions & 0 deletions packages/api/src/utils/acceptHeader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import {ResponseFormat} from "../interfaces.js";

enum MediaType {
json = "application/json",
ssz = "application/octet-stream",
}

const MEDIA_TYPES: {
[K in ResponseFormat]: MediaType;
} = {
json: MediaType.json,
ssz: MediaType.ssz,
};

function responseFormatFromMediaType(mediaType: MediaType): ResponseFormat {
switch (mediaType) {
default:
case MediaType.json:
return "json";
case MediaType.ssz:
return "ssz";
}
}

export function writeAcceptHeader(format?: ResponseFormat): MediaType {
return format == undefined ? MEDIA_TYPES["json"] : MEDIA_TYPES[format];
}

export function parseAcceptHeader(accept?: string): ResponseFormat {
// Use json by default.
if (!accept) {
return "json";
}

const mediaTypes = Object.values(MediaType);

// Respect Quality Values per RFC-9110
// Acceptable mime-types are comma separated with optional whitespace
return responseFormatFromMediaType(
accept
.toLowerCase()
.split(",")
.map((x) => x.trim())
.reduce(
(best: [number, MediaType], current: string): [number, MediaType] => {
// An optional `;` delimiter is used to separate the mime-type from the weight
// Normalize here, using 1 as the default qvalue
const quality = current.includes(";") ? current.split(";") : [current, "q=1"];

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

// If the mime type isn't acceptable, move on to the next entry
if (!mediaTypes.includes(mediaType)) {
return best;
}

// Otherwise, the portion after the semicolon has optional whitespace and the constant prefix "q="
const weight = quality[1].trim();
if (!weight.startsWith("q=")) {
// If the format is invalid simply move on to the next entry
return best;
}

const qvalue = +weight.replace("q=", "");
if (isNaN(qvalue) || qvalue > 1 || qvalue <= 0) {
// If we can't convert the qvalue to a valid number, move on
return best;
}

if (qvalue < best[0]) {
// This mime type is not preferred
return best;
}

// This mime type is preferred
return [qvalue, mediaType];
},
[0, MediaType.json]
)[1]
);
}
37 changes: 37 additions & 0 deletions packages/api/test/unit/utils/acceptHeader.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {expect} from "chai";
import {parseAcceptHeader} from "../../../src/utils/acceptHeader.js";
import {ResponseFormat} from "../../../src/interfaces.js";

describe("utils / acceptHeader", () => {
describe("parseAcceptHeader", () => {
const testCases: {header: string | undefined; expected: ResponseFormat}[] = [
{header: undefined, expected: "json"},
{header: "application/json", expected: "json"},
{header: "application/octet-stream", expected: "ssz"},
{header: "application/invalid", expected: "json"},
{header: "application/invalid;q=1,application/octet-stream;q=0.1", expected: "ssz"},
{header: "application/octet-stream;q=0.5,application/json;q=1", expected: "json"},
{header: "application/octet-stream;q=1,application/json;q=0.1", expected: "ssz"},
{header: "application/octet-stream,application/json;q=0.1", expected: "ssz"},
{header: "application/octet-stream;,application/json;q=0.1", expected: "json"},
{header: "application/octet-stream;q=2,application/json;q=0.1", expected: "json"},
{header: "application/octet-stream;q=invalid,application/json;q=0.1", expected: "json"},
{header: "application/octet-stream;q=invalid,application/json;q=0.1", expected: "json"},
{header: "application/octet-stream ; q=0.5 , application/json ; q=1", expected: "json"},
{header: "application/octet-stream ; q=1 , application/json ; q=0.1", expected: "ssz"},
{header: "application/octet-stream;q=1,application/json;q=0.1", expected: "ssz"},

// 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
// specific requirement from the spec. In this case, last wins.
{header: "application/octet-stream;q=1,application/json;q=1", expected: "json"},
{header: "application/json;q=1,application/octet-stream;q=1", expected: "ssz"},
];

for (const testCase of testCases) {
it(`should correctly parse the header ${testCase.header}`, () => {
expect(parseAcceptHeader(testCase.header)).to.equal(testCase.expected);
});
}
});
});
6 changes: 3 additions & 3 deletions packages/beacon-node/src/api/impl/debug/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {routes, ServerApi} from "@lodestar/api";
import {routes, ServerApi, ResponseFormat} from "@lodestar/api";
import {resolveStateId} from "../beacon/state/utils.js";
import {ApiModules} from "../types.js";
import {isOptimisticBlock} from "../../../util/forkChoice.js";
Expand Down Expand Up @@ -36,7 +36,7 @@ export function getDebugApi({chain, config}: Pick<ApiModules, "chain" | "config"
return {data: nodes};
},

async getState(stateId: string | number, format?: routes.debug.StateFormat) {
async getState(stateId: string | number, format?: ResponseFormat) {
const {state} = await resolveStateId(chain, stateId, {allowRegen: true});
if (format === "ssz") {
// Casting to any otherwise Typescript doesn't like the multi-type return
Expand All @@ -47,7 +47,7 @@ export function getDebugApi({chain, config}: Pick<ApiModules, "chain" | "config"
}
},

async getStateV2(stateId: string | number, format?: routes.debug.StateFormat) {
async getStateV2(stateId: string | number, format?: ResponseFormat) {
const {state} = await resolveStateId(chain, stateId, {allowRegen: true});
if (format === "ssz") {
// Casting to any otherwise Typescript doesn't like the multi-type return
Expand Down

0 comments on commit 299406f

Please sign in to comment.