Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 8 additions & 10 deletions lib/types/request.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export interface RequestAuth<
* set to false.
*/
isAuthorized: boolean;
/** true if the request has been authenticated via the `server.inject()` `auth` option, otherwise `undefined`. */
isInjected?: boolean | undefined;
/** the route authentication mode. */
mode: AuthMode;
/** the name of the strategy used. */
Expand Down Expand Up @@ -250,7 +252,7 @@ export interface RequestLog {
}

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

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

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

AuthUser: UserCredentials;
Expand Down Expand Up @@ -303,11 +305,7 @@ export type ReqRef = Partial<Record<keyof ReqRefDefaults, unknown>>;
/**
* Utilities for merging request refs and other things
*/
export type MergeType<T, U> = {
[K in keyof T]: K extends keyof U
? U[K]
: T[K];
};
export type MergeType<T, U> = Omit<T, keyof U> & U;

export type MergeRefs<T extends ReqRef> = MergeType<ReqRefDefaults, T>;

Expand Down Expand Up @@ -441,7 +439,7 @@ export interface Request<Refs extends ReqRef = ReqRefDefaults> extends Podium {
/**
* Same as pre but represented as the response object created by the pre method.
*/
readonly preResponses: Record<string, any>;
readonly preResponses: Record<string, unknown>;

/**
* By default the object outputted from node's URL parse() method.
Expand Down Expand Up @@ -474,7 +472,7 @@ export interface Request<Refs extends ReqRef = ReqRefDefaults> extends Podium {
/**
* An object containing parsed HTTP state information (cookies) where each key is the cookie name and value is the matching cookie content after processing using any registered cookie definition.
*/
readonly state: Record<string, any>;
readonly state: Record<string, unknown>;

/**
* The parsed request URI.
Expand Down
4 changes: 2 additions & 2 deletions lib/types/route.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ export interface RouteOptionsPreObject<Refs extends ReqRef = ReqRefDefaults> {
/**
* key name used to assign the response of the method to in request.pre and request.preResponses.
*/
assign?: keyof Refs['Pres'] | undefined;
assign?: keyof MergeRefs<Refs>['Pres'] | undefined;
/**
* A failAction value which determine what to do when a pre-handler method throws an error. If assign is specified and the failAction setting is not 'error', the error will be assigned.
*/
Expand Down Expand Up @@ -978,5 +978,5 @@ export interface ServerRoute<Refs extends ReqRef = ReqRefDefaults> {
/**
* route custom rules object. The object is passed to each rules processor registered with server.rules(). Cannot be used if route.options.rules is defined.
*/
rules?: Refs['Rules'] | undefined;
rules?: MergeRefs<Refs>['Rules'] | undefined;
}
246 changes: 237 additions & 9 deletions test/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as CatboxMemory from '@hapi/catbox-memory';

import {
Plugin,
ReqRef,
Request,
RequestRoute,
ResponseToolkit,
Expand Down Expand Up @@ -68,20 +69,20 @@ interface RequestDecorations {
prefix: string[];
},
AuthUser: {
id: string,
name: string
email: string
id: string;
name: string;
email: string;
},
AuthCredentialsExtra: {
test: number
}
test: number;
},
AuthApp: {
key: string
name: string
key: string;
name: string;
},
AuthArtifactsExtra: {
some: string
thing: number
some: string;
thing: number;
}
}

Expand Down Expand Up @@ -287,3 +288,230 @@ server.decorate('server', 'obj3_1', { func5: theFunc });
// Error when extending on server with objects
// @ts-expect-error Lab does not support overload errors
check.error(() => server.decorate('server', 'obj3_1', { func5: theFunc }, { apply: true, extend: true }));

// Issue #4561 - Generic Request<Refs> should resolve augmented ReqRefDefaults auth properties

interface ExtraCred {
extra_id: string;
}

interface UserProfile {
id: string;
}

declare module '../..' {
interface ReqRefDefaults {
AuthCredentialsExtra: Partial<ExtraCred>;
}
}

// Generic route (no custom refs) should see augmented UserCredentials
const genericAuthRoute: ServerRoute = {
method: 'GET',
path: '/auth-check',
handler: (request, h) => {

check.type<string>(request.auth.credentials.user!.someId);
check.type<string>(request.auth.credentials.user!.someName);

const credIsAny: IsAny<typeof request.auth.credentials> = false;

return 'ok';
}
};

// Generic function should see augmented credentials from ReqRefDefaults
export function processAuthGeneric<Refs extends ReqRef>(req: Request<Refs>): void {

if (req.auth.isAuthenticated && req.auth.credentials.extra_id) {
check.type<string | undefined>(req.auth.credentials.extra_id);
}
}

// Non-generic Request should also see augmented credentials
export function processAuthConcrete(req: Request): void {

if (req.auth.isAuthenticated && req.auth.credentials.extra_id) {
check.type<string | undefined>(req.auth.credentials.extra_id);
}

// credentials should NOT resolve to `any`
const credIsAny: IsAny<typeof req.auth.credentials> = false;
const artifactsIsAny: IsAny<typeof req.auth.artifacts> = false;
}

// Generic function should accept Request with specific route refs
interface SpecificRouteRefs {
Params: { id: string };
}

export function callWithSpecificRefs(req: Request<SpecificRouteRefs>): void {

processAuthGeneric(req);
}

// =============================================================================
// ReqRef System Issue Tests
// Each section demonstrates a specific weakness in the current type system.
// These tests produce VISIBLE compiler errors to demonstrate each problem.
// =============================================================================

// -----------------------------------------------------------------------------
// ISSUE 1: Direct Refs['Key'] access bypasses MergeRefs (route.d.ts:361)
//
// RouteOptionsPreObject.assign uses `keyof Refs['Pres']` instead of
// `keyof MergeRefs<Refs>['Pres']`. When the user doesn't explicitly provide
// `Pres` in their Refs, `Refs['Pres']` is `unknown` (from ReqRef's
// Partial<Record<..., unknown>>), so `keyof unknown` is `never`.
// This means `assign` is impossible unless Pres is explicitly provided.
// -----------------------------------------------------------------------------

// This should compile — the user only customizes Params, and the default
// Pres (Record<string, any>) should allow any string for `assign`.
// ERROR: Type '"user"' is not assignable to type 'never'.
const issuePreAssign: ServerRoute<{ Params: { id: string } }> = {
method: 'GET',
path: '/users/{id}',
options: {
pre: [
{
method: (request, h) => ({ name: 'test' }),
assign: 'user' // TS ERROR — should work
}
],
handler: (request, h) => 'ok'
}
};

// -----------------------------------------------------------------------------
// 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.
// -----------------------------------------------------------------------------

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[]
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.
// -----------------------------------------------------------------------------

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
const auth: number = request.headers.authorization;

// 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`.
// -----------------------------------------------------------------------------

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
const page: number = request.query.page;
// @ts-expect-error - query values are string | string[] | undefined, not boolean[]
const wat: boolean[] = request.query.anything;

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

return 'ok';
}
};

// -----------------------------------------------------------------------------
// ISSUE 5: Request<CustomRefs> not assignable to Request<ReqRefDefaults>
//
// A function taking Request (no generic) can't accept Request<{ Params: ... }>
// even though the custom refs only NARROW a property. Users are forced to
// choose between generic (accepts all) or concrete (sees defaults).
// -----------------------------------------------------------------------------

export function concreteHelper(req: Request): string | undefined {

if (req.auth.credentials.extra_id) {
return req.auth.credentials.extra_id;
}

return undefined;
}

interface MyRouteRefs {
Params: { id: string };
Query: { expand: string };
}

// KNOWN LIMITATION: Request<MyRouteRefs> is not assignable to Request<ReqRefDefaults>
// because TypeScript checks generic interface compatibility invariantly when
// the generic appears in contravariant positions (e.g. lifecycle method parameters).
// Workaround: use a generic function like processAuthGeneric<Refs> above instead
// of concrete Request (no generic) for helper functions that need to accept
// requests with different Refs.
export function issueConcreteVsGeneric(req: Request<MyRouteRefs>): void {

// @ts-expect-error - Known TS limitation: Request<CustomRefs> not assignable to Request<ReqRefDefaults>
concreteHelper(req);
}

// -----------------------------------------------------------------------------
// ISSUE 6: state and preResponses are not extensible through ReqRef
//
// These properties use hardcoded Record<string, any> and are NOT wired
// through InternalRequestDefaults/ReqRef, so users can't type them.
// -----------------------------------------------------------------------------

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

// FIXED: state is now Record<string, unknown> — requires type narrowing
// @ts-expect-error - state values are unknown, not directly assignable to number
const session: number = request.state.session;

// FIXED: state is no longer `any`
const stateIsAny: IsAny<typeof request.state.session> = false;

// FIXED: preResponses is no longer `any`
const preRespIsAny: IsAny<typeof request.preResponses.myPre> = false;

return 'ok';
}
};
Loading