Skip to content

Commit

Permalink
fix: add createHttpClient back to app instance (#5383)
Browse files Browse the repository at this point in the history
security plugin need it to create a new httpClient for SSRF

https://github.com/eggjs/egg/blob/a612e806019402aa217a1562b5ad847a308e843b/lib/egg.js#L293
https://github.com/eggjs/security/blob/e3408408adec5f8d009d37f75126ed082481d0ac/lib/extend/safe_curl.js#L21

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced TypeScript type definitions for Context, Request, Response,
and HttpClient interfaces.
- Added `createHttpClient` method to create HTTP client instances with
custom options.

- **Improvements**
- Improved type safety and configuration flexibility for core framework
interfaces.
  - Streamlined HTTP client configuration management.

- **Testing**
  - Added test coverage for HTTP client instance creation and usage.
- Updated test cases to enhance type safety for the app parameter in
loader tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
fengmk2 authored Jan 14, 2025
1 parent c098596 commit e5a697e
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 128 deletions.
18 changes: 16 additions & 2 deletions src/app/extend/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import type Request from './request.js';
import type Response from './response.js';
import type Helper from './helper.js';

import './context.types.js';

const HELPER = Symbol('ctx helper');
const LOCALS = Symbol('ctx locals');
const LOCALS_LIST = Symbol('ctx localsList');
Expand Down Expand Up @@ -306,3 +304,19 @@ export default class Context extends EggCoreContext {
this.response.realStatus = val;
}
}

declare module '@eggjs/core' {
// add Context overrides types
interface Context {
curl(url: HttpClientRequestURL, options?: HttpClientRequestOptions): ReturnType<HttpClient['request']>;
get router(): Router;
set router(val: Router);
get helper(): Helper;
get httpclient(): HttpClient;
get httpClient(): HttpClient;
getLogger(name: string): EggLogger;
get logger(): EggLogger;
get coreLogger(): EggLogger;
get locals(): Record<string, any>;
}
}
24 changes: 0 additions & 24 deletions src/app/extend/context.types.ts

This file was deleted.

13 changes: 11 additions & 2 deletions src/app/extend/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ const HOST = Symbol('request host');
const IPS = Symbol('request ips');
const RE_ARRAY_KEY = /[^\[\]]+\[\]$/;

import './request.types.js';

export default class Request extends EggCoreRequest {
declare app: Application;
declare ctx: Context;
Expand Down Expand Up @@ -266,6 +264,17 @@ export default class Request extends EggCoreRequest {
}
}

declare module '@eggjs/core' {
// add Request overrides types
interface Request {
body: any;
get acceptJSON(): boolean;
get query(): Record<string, string>;
set query(obj: Record<string, string>);
get queries(): Record<string, string[]>;
}
}

function firstValue(value: string | string[]) {
if (Array.isArray(value)) {
value = value[0];
Expand Down
10 changes: 0 additions & 10 deletions src/app/extend/request.types.ts

This file was deleted.

10 changes: 8 additions & 2 deletions src/app/extend/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { Response as KoaResponse } from '@eggjs/core';

const REAL_STATUS = Symbol('response realStatus');

import './response.types.js';

export default class Response extends KoaResponse {
/**
* Get or set a real status code.
Expand Down Expand Up @@ -36,3 +34,11 @@ export default class Response extends KoaResponse {
this[REAL_STATUS] = status;
}
}

declare module '@eggjs/core' {
// add Response overrides types
interface Response {
get realStatus(): number;
set realStatus(status: number);
}
}
7 changes: 0 additions & 7 deletions src/app/extend/response.types.ts

This file was deleted.

15 changes: 11 additions & 4 deletions src/lib/core/httpclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import {
HttpClient as RawHttpClient,
RequestURL as HttpClientRequestURL,
RequestOptions,
ClientOptions as HttpClientOptions,
} from 'urllib';
import { ms } from 'humanize-ms';
import type { EggApplicationCore } from '../egg.js';

export type {
HttpClientResponse,
RequestURL as HttpClientRequestURL,
ClientOptions as HttpClientOptions,
} from 'urllib';

export interface HttpClientRequestOptions extends RequestOptions {
Expand All @@ -19,12 +21,17 @@ export interface HttpClientRequestOptions extends RequestOptions {
export class HttpClient extends RawHttpClient {
readonly #app: EggApplicationCore & { tracer?: any };

constructor(app: EggApplicationCore) {
constructor(app: EggApplicationCore, options: HttpClientOptions = {}) {
normalizeConfig(app);
const config = app.config.httpclient;
super({
defaultArgs: config.request,
});
const initOptions: HttpClientOptions = {
...options,
defaultArgs: {
...config.request,
...options.defaultArgs,
},
};
super(initOptions);
this.#app = app;
}

Expand Down
34 changes: 29 additions & 5 deletions src/lib/egg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import type { EggAppConfig } from './types.js';
import { create as createMessenger, IMessenger } from './core/messenger/index.js';
import { ContextHttpClient } from './core/context_httpclient.js';
import {
HttpClient, type HttpClientRequestOptions, type HttpClientRequestURL, type HttpClientResponse,
HttpClient,
type HttpClientRequestOptions, type HttpClientRequestURL, type HttpClientResponse,
type HttpClientOptions,
} from './core/httpclient.js';
import { createLoggers } from './core/logger.js';
import {
Expand All @@ -43,8 +45,6 @@ import { BaseHookClass } from './core/base_hook_class.js';
import type { EggApplicationLoader } from './loader/index.js';
import { getSourceDirname } from './utils.js';

import './egg.types.js';

const EGG_PATH = Symbol.for('egg#eggPath');

export interface EggApplicationCoreOptions extends Omit<EggCoreOptions, 'baseDir'> {
Expand Down Expand Up @@ -387,14 +387,22 @@ export class EggApplicationCore extends EggCore {
return await this.httpClient.request<T>(url, options);
}

/**
* Create a new HttpClient instance with custom options
* @param {Object} [options] HttpClient init options
*/
createHttpClient(options?: HttpClientOptions): HttpClient {
return new this.HttpClient(this, options);
}

/**
* HttpClient instance
* @see https://github.com/node-modules/urllib
* @member {HttpClient}
*/
get httpClient() {
get httpClient(): HttpClient {
if (!this.#httpClient) {
this.#httpClient = new this.HttpClient(this);
this.#httpClient = this.createHttpClient();
}
return this.#httpClient;
}
Expand Down Expand Up @@ -680,3 +688,19 @@ export class EggApplicationCore extends EggCore {
return context;
}
}

declare module '@eggjs/core' {
// add EggApplicationCore overrides types
interface EggCore {
inspect(): any;
get currentContext(): EggContext | undefined;
ctxStorage: AsyncLocalStorage<EggContext>;
get logger(): EggLogger;
get coreLogger(): EggLogger;
getLogger(name: string): EggLogger;
createHttpClient(options?: HttpClientOptions): HttpClient;
HttpClient: typeof HttpClient;
get httpClient(): HttpClient;
curl<T = any>(url: HttpClientRequestURL, options?: HttpClientRequestOptions): Promise<HttpClientResponse<T>>;
}
}
15 changes: 0 additions & 15 deletions src/lib/egg.types.ts

This file was deleted.

53 changes: 12 additions & 41 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ export interface CustomLoaderConfig extends Omit<FileLoaderOptions, 'inject' | '
loadunit?: boolean;
}

export interface HttpClientConfig {
/** Request timeout */
timeout?: number;
/** Default request args for httpclient */
request?: HttpClientRequestOptions;
/**
* @deprecated keep compatible with egg 3.x, no more used
*/
useHttpClientNext?: boolean;
}

export interface EggAppConfig extends EggCoreAppConfig {
workerStartTimeout: number;
baseDir: string;
Expand Down Expand Up @@ -141,47 +152,7 @@ export interface EggAppConfig extends EggCoreAppConfig {
};

/** Configuration of httpclient in egg. */
httpclient: {
/** Request timeout */
timeout?: number;
/** Default request args for httpclient */
request?: HttpClientRequestOptions;
/**
* @deprecated keep compatible with egg 3.x, no more used
*/
useHttpClientNext?: boolean;
};

// development: {
// /**
// * dirs needed watch, when files under these change, application will reload, use relative path
// */
// watchDirs: string[];
// /**
// * dirs don't need watch, including subdirectories, use relative path
// */
// ignoreDirs: string[];
// /**
// * don't wait all plugins ready, default is true.
// */
// fastReady: boolean;
// /**
// * whether reload on debug, default is true.
// */
// reloadOnDebug: boolean;
// /**
// * whether override default watchDirs, default is false.
// */
// overrideDefault: boolean;
// /**
// * whether override default ignoreDirs, default is false.
// */
// overrideIgnore: boolean;
// /**
// * whether to reload, use https://github.com/sindresorhus/multimatch
// */
// reloadPattern: string[] | string;
// };
httpclient: HttpClientConfig;

/**
* customLoader config
Expand Down
19 changes: 19 additions & 0 deletions test/lib/core/httpclient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,4 +634,23 @@ describe('test/lib/core/httpclient.test.ts', () => {
assert(res.status === 200);
});
});

describe('app.createHttpClient(options)', () => {
let app: MockApplication;
before(() => {
app = createApp('apps/httpclient-retry');
return app.ready();
});
after(() => app.close());

it('should work', async () => {
const client1 = app.createHttpClient();
const client2 = app.createHttpClient();
assert.notEqual(client1, client2);
const res = await client1.request(url, {
method: 'GET',
});
assert.equal(res.status, 200);
});
});
});
Loading

0 comments on commit e5a697e

Please sign in to comment.