Skip to content

fix: broaden Query/Params/Headers ref defaults to unknown #4578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
damusix wants to merge 1 commit into
base: master
Choose a base branch
from
+102 −40
Open

fix: broaden Query/Params/Headers ref defaults to unknown #4578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/types/request.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ export interface RequestLog {
}

export interface RequestQuery {
[key: string]: string | string[] | undefined;
[key: string]: unknown;
}

/**
Expand All @@ -271,9 +271,9 @@ export interface InternalRequestDefaults {

Payload: stream.Readable | Buffer | string | object;
Query: RequestQuery;
Params: Record<string, string>;
Params: Record<string, unknown>;
Pres: Record<string, any>;
Headers: Record<string, string | string[] | undefined>;
Headers: Record<string, unknown>;
RequestApp: RequestApplicationState;

AuthUser: UserCredentials;
Expand Down
124 changes: 93 additions & 31 deletions test/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as CatboxMemory from '@hapi/catbox-memory';
import {
Plugin,
ReqRef,
ReqRefDefaults,
Request,
RequestRoute,
ResponseToolkit,
Expand Down Expand Up @@ -106,7 +107,7 @@ const route: ServerRoute<RequestDecorations> = {

request.app.word = 'x';

check.type<Record<string, string>>(request.params);
check.type<Record<string, unknown>>(request.params);
check.type<number>(request.server.app.multi!);
check.type<string[]>(request.route.settings.app!.prefix);

Expand Down Expand Up @@ -383,73 +384,61 @@ const issuePreAssign: ServerRoute<{ Params: { id: string } }> = {
}
};

// -----------------------------------------------------------------------------
// ISSUE 2: Params defaults to Record<string, any> — allows unsafe access
//
// URL path params are ALWAYS strings at runtime (before Joi validation), but
// the default type Record<string, any> means TypeScript allows anything.
// These assignments should all be errors but none are.
// -----------------------------------------------------------------------------
// ISSUE 2: Params default is `Record<string, unknown>` — Joi-driven
// conversion can change the runtime type, so callsites must narrow.

const issueParamsAny: ServerRoute = {
method: 'GET',
path: '/items/{id}',
handler: (request, h) => {

// FIXED: Params now correctly typed as Record<string, string>
// @ts-expect-error - params are strings, not numbers
const id: number = request.params.id;
// @ts-expect-error - params are strings, not boolean[]
// @ts-expect-error - unknown is not number
const idAsNum: number = request.params.id;
// @ts-expect-error - unknown is not string (compiled pre-fix when Params was Record<string, string>)
const idAsStr: string = request.params.id;
// @ts-expect-error - unknown is not boolean[]
const wat: boolean[] = request.params.id;

// FIXED: params is no longer `any`
const paramsIsAny: IsAny<typeof request.params.id> = false;

return 'ok';
}
};

// -----------------------------------------------------------------------------
// ISSUE 3: Headers defaults to Record<string, any>
//
// Node's http.IncomingHttpHeaders types headers as string | string[] | undefined.
// The Record<string, any> default loses this.
// -----------------------------------------------------------------------------
// ISSUE 3: Headers default is `Record<string, unknown>` — header validation
// can transform values (e.g. `x-date` → Date).

const issueHeadersAny: ServerRoute = {
method: 'GET',
path: '/headers',
handler: (request, h) => {

// FIXED: Headers now correctly typed as Record<string, string | string[] | undefined>
// @ts-expect-error - headers are string | string[] | undefined, not number
// @ts-expect-error - unknown is not number
const auth: number = request.headers.authorization;
// @ts-expect-error - unknown is not string (issue #4563 — was assignable in 21.4.4)
const xCustom: string = request.headers['x-custom-header'];

// FIXED: headers is no longer `any`
const headersIsAny: IsAny<typeof request.headers.authorization> = false;

return 'ok';
}
};

// -----------------------------------------------------------------------------
// ISSUE 4: Default RequestQuery has [key: string]: any index signature
//
// Without a Query override, any access on request.query is `any`.
// -----------------------------------------------------------------------------
// ISSUE 4: Query default is `[key: string]: unknown` — the configured parser
// (e.g. `qs` yields objects) and validation determine the runtime shape.

const issueQueryAny: ServerRoute = {
method: 'GET',
path: '/search',
handler: (request, h) => {

// FIXED: Query now correctly typed as Record<string, string | string[] | undefined>
// @ts-expect-error - query values are string | string[] | undefined, not number
// @ts-expect-error - unknown is not number
const page: number = request.query.page;
// @ts-expect-error - query values are string | string[] | undefined, not boolean[]
// @ts-expect-error - unknown is not string
const q: string = request.query.q;
// @ts-expect-error - unknown is not boolean[]
const wat: boolean[] = request.query.anything;

// FIXED: query is no longer `any`
const queryIsAny: IsAny<typeof request.query.page> = false;

return 'ok';
Expand Down Expand Up @@ -515,3 +504,76 @@ const issueStateAny: ServerRoute = {
return 'ok';
}
};

// Headers/Params can be widened via global augmentation.

type Extends<A, B> = A extends B ? true : false;

interface ParsedHeaders {
[key: string]: string | string[] | Date | number | undefined;
}
const _widensHeaders: Extends<ParsedHeaders, ReqRefDefaults['Headers']> = true;

interface ConvertedParams { [key: string]: string | number; }
const _widensParams: Extends<ConvertedParams, ReqRefDefaults['Params']> = true;

// Global Query augmentation must compile under `unknown` defaults and
// propagate through every generic position that reads MergeRefs<Refs>['Query'].

declare module '../..' {
interface ReqRefDefaults {
Query: {
[key: string]: string | string[] | Record<string, object> | undefined;
};
}
}

type AugmentedQuery = string | string[] | Record<string, object> | undefined;

// ServerRoute with no generic — augmented Query flows in via Request<ReqRefDefaults>.
const augNoGeneric: ServerRoute = {
method: 'GET',
path: '/aug-no-generic',
handler: (request, h) => {

check.type<AugmentedQuery>(request.query.anything);
return 'ok';
}
};
server.route(augNoGeneric);

// ServerRoute with partial Refs override — Query falls through MergeRefs as augmented.
const augPartialRefs: ServerRoute<{ Params: { id: string } }> = {
method: 'GET',
path: '/aug-partial/{id}',
handler: (request, h) => {

check.type<AugmentedQuery>(request.query.foo);
check.type<string>(request.params.id);
return 'ok';
}
};
server.route(augPartialRefs);

// Lifecycle.Method (no generic) — same augmented Query.
const augLifecycle: Lifecycle.Method = (request, h) => {

check.type<AugmentedQuery>(request.query.x);
return 'ok';
};
augLifecycle.length; // keep referenced

// Per-route Query override still replaces the augmented default outright.
const qsParsedRoute: ServerRoute<{ Query: { filter: Record<string, unknown>; q?: string } }> = {
method: 'GET',
path: '/qs',
handler: (request, h) => {

check.type<Record<string, unknown>>(request.query.filter);
check.type<string | undefined>(request.query.q);

return 'ok';
}
};

server.route(qsParsedRoute);
12 changes: 6 additions & 6 deletions typescript.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ Defines every customizable key and its default type:
| Key | Default Type | Controls |
| ---------------------- | ------------------------------------------------- | ---------------------------------------------- |
| `Payload` | `stream.Readable \| Buffer \| string \| object` | `request.payload` |
| `Query` | `Record<string, string \| string[] \| undefined>` | `request.query` |
| `Params` | `Record<string, string>` | `request.params` |
| `Query` | `RequestQuery` (`Record<string, unknown>`) | `request.query` |
| `Params` | `Record<string, unknown>` | `request.params` |
| `Pres` | `Record<string, any>` | `request.pre` |
| `Headers` | `Record<string, string \| string[] \| undefined>` | `request.headers` |
| `Headers` | `Record<string, unknown>` | `request.headers` |
| `RequestApp` | `RequestApplicationState` | `request.app` |
| `AuthUser` | `UserCredentials` | `request.auth.credentials.user` |
| `AuthApp` | `AppCredentials` | `request.auth.credentials.app` |
Expand Down Expand Up @@ -117,7 +117,7 @@ const route: ServerRoute<MyRefs> = {

### Params

Default: `Record<string, string>`. URL path parameters are always strings at runtime (before validation), so the default type reflects this.
Default: `Record<string, unknown>`. Raw path params are strings; Joi conversion can change that at runtime, so the default forces narrowing.

```typescript
// Override with specific param names
Expand All @@ -136,7 +136,7 @@ const route: ServerRoute<{ Params: { userId: string; postId: string } }> = {

### Query

Default: `Record<string, string | string[] | undefined>`. Query params may be strings, arrays (repeated keys), or absent.
Default: `RequestQuery`, an augmentable interface with `[key: string]: unknown`. The shape depends on the configured parser (default produces `string | string[] | undefined`; `qs` yields nested objects) and on validation. Narrow per-route, or augment `RequestQuery` globally.

```typescript
interface SearchQuery {
Expand Down Expand Up @@ -185,7 +185,7 @@ const route: ServerRoute<{ Payload: CreateUserPayload }> = {

### Headers

Default: `Record<string, string | string[] | undefined>`. Matches Node's `http.IncomingHttpHeaders` behavior. Override only if you need to narrow specific header names.
Default: `Record<string, unknown>`. Raw headers match Node's `http.IncomingHttpHeaders` (`string | string[] | undefined`); validation can produce anything, so the default forces narrowing.


### RequestApp
Expand Down
Loading