feat(provider): pass request headers through gateway#322
Merged
Conversation
- Add passRequestHeaders array field to gateway config - Support forwarding specific request headers to providers - Automatically include user-agent when passRequestUserAgent enabled - Add DefaultProviderRequestHeaders type for type safety - Update GetNodeListParams to accept requestHeaders Enables selective forwarding of HTTP headers from gateway requests to upstream provider subscriptions.
- Refactor requestCacheableResource to accept headers and cache key - Add determineRequestHeaders method to compute final headers - Update all providers to use new header-based API - Replace requestUserAgent parameter with requestHeaders object - Move cache key generation to static method for consistency - Apply header filtering based on passGatewayRequestHeaders Changes affect ClashProvider, TrojanProvider, ShadowsocksProvider, ShadowsocksrProvider, SsdProvider, V2rayNProvider, and ShadowsocksJsonProvider to use consistent header handling.
- Add requestHeaders and cacheKey parameters to test calls - Update mock assertions to use sinon.match for header checking - Replace exact match assertions with flexible header matching - Add required parameters to all provider test cases Updates tests to match refactored provider API signatures.
- Fix cache key generation to include all headers, not just user-agent - Update determineRequestUserAgent to properly require requestUserAgent - Fix determineRequestHeaders to always include user-agent in result - Add case-insensitive header key normalization - Enhance requestCacheableResource with auto-generated cache keys - Add comprehensive JSDoc documentation for header methods Addresses critical issues where user-agent could be lost from headers and cache keys could collide when using different headers.
- Update all providers to use new getResourceCacheKey signature - Pass headers object directly instead of concatenating user-agent - Ensures cache keys properly include all header variations - Fixes cache collision issue where different headers used same key Updates ClashProvider, TrojanProvider, ShadowsocksSubscribeProvider, ShadowsocksrSubscribeProvider, SsdProvider, and ShadowsocksJsonSubscribeProvider.
- Update GetSubscriptionUserInfoFunction to accept GetNodeListParams - Pass requestHeaders parameter in all provider implementations - Ensures API consistency between getNodeList and getSubscriptionUserInfo - Allows custom headers to be forwarded for subscription info requests Fixes API inconsistency where getSubscriptionUserInfo did not support the requestHeaders parameter that getNodeList accepted.
Implement getNodeListV2 method that returns both node list and subscription userinfo in a single call. This eliminates the need for separate method calls and improves efficiency by using shared helper functions where available. - Add GetNodeListV2Result and GetNodeListV2Function types - Implement default getNodeListV2 in Provider base class - Add optimized overrides in all subscription-supporting providers - Update subscriptions command to use getNodeListV2 - Update artifact generation to use getNodeListV2 - Update check command to use getNodeListV2 SubscriptionUserinfo is optional in the return object, removing the need for supportGetSubscriptionUserInfo flag checks in consumer code. All existing methods remain unchanged for backward compatibility.
Replace the default implementation that composed getNodeList and getSubscriptionUserInfo with an abstract method. This ensures each provider must implement getNodeListV2 to optimize data fetching when both node list and subscription info come from the same source. Removes unused imports GetNodeListParams and GetNodeListV2Result from the base class.
Add getNodeListV2 implementations to CustomProvider, ShadowsocksJsonSubscribeProvider, and V2rayNSubscribeProvider. These providers don't support subscription user info, so their implementations simply wrap getNodeList results. All provider subclasses now satisfy the abstract getNodeListV2 method requirement from the base Provider class.
Update test to use distinct cache keys to prevent collisions between test cases. Removes a test case that was validating error handling that is now redundant.
Clean up formatting in ShadowsocksrSubscribeProvider and V2rayNSubscribeProvider for consistency. Simplify type assertions and adjust line wrapping.
- Upgrade eslint from ^8.57.1 to ^9.28.0 - Update @surgio/eslint-config-surgio from ^1.0.6 to ^2.0.0 - Replace @typescript-eslint/eslint-plugin and @typescript-eslint/parser with typescript-eslint package - Replace eslint-plugin-import with eslint-plugin-import-x - Update related packages: eslint-config-prettier, eslint-plugin-prettier, eslint-import-resolver-typescript - Add globals package for ESLint 9 flat config support - Remove @types/eslint (no longer needed with ESLint 9) Align with ESLint 9 ecosystem requirements.
- Delete .eslintrc.js and .eslintignore (ESLint 8 format) - Add eslint.config.mjs with ESLint 9 flat config - Configure separate blocks for TypeScript, test, and JavaScript files - Use typescript-eslint, eslint-plugin-import-x, and eslint-plugin-prettier - Define ignores, language options, plugins, and rules using flat config API Migrate from legacy eslintrc to flat config format required by ESLint 9.
- Replace ESLint 8 options (useEslintrc, extensions, baseConfig.extends) - Use overrideConfigFile and overrideConfig for flat config API - Await formatter.format() result (ESLint 9 returns Promise) - Import surgioConfig directly instead of using require.resolve() - Add rootDir to tsconfig.build.json to output files directly to build/ Update ESLint initialization to use flat config API required by ESLint 9.
- Replace eslint-disable comments with unused catch binding syntax (_err) - Fix regex escaping: \. should be . in character classes - Remove unnecessary eslint-disable-next-line prefer-const comments Auto-fixed by ESLint 9 with updated rules configuration.
- Add entries for Thumbs.db, various log files, and environment configurations. - Include build output files and IDE-specific files. - Remove redundant entries and ensure coverage and test directories are properly ignored.
- Update DefaultProviderRequestHeaders to use IncomingHttpHeaders instead of custom Record type - Update requestHeaders parameter in GetNodeListParams to use IncomingHttpHeaders Provides better type safety and standardization for HTTP headers handling.
…ayRequestHeaders - Replace passGatewayRequestUserAgent property with passGatewayRequestHeaders array - Add determineRequestUserAgent and determineRequestHeaders methods for header management - Include deprecation warning for legacy passRequestUserAgent configuration - Update ClashProvider tests to use new passGatewayRequestHeaders approach BREAKING CHANGE: passGatewayRequestUserAgent property has been removed. Use passGatewayRequestHeaders array instead.
…ayRequestHeaders - Remove deprecated DEP011 warning - Add PASS_GATEWAY_REQUEST_HEADERS_WHITELIST constant - Enhance header processing in base Provider class - Remove automatic user-agent injection logic
- Fix typo in interfaces and types - Update SubsciptionCacheItem and GetNodeListV2Result - Ensure type consistency across the codebase
- Add subscriptionUserInfo property to Artifact class - Add subscriptionUserinfoMap for per-provider tracking - Improve subscription info storage and retrieval logic
- Fix subscriptionUserInfo spelling across all providers - Update method signatures and variable names - Ensure consistent API usage across provider implementations
- Fix subscriptionUserInfo spelling in CLI command - Ensure command layer uses correct type names
- Remove agentkeepalive dependency and use native http/https.Agent - Update package.json to remove external dependency - Add pnpm-workspace.yaml for modern package management - Maintain same keep-alive functionality with native agents
- Make passGatewayRequestHeaders private in Provider base class - Add type safety with satisfies operator in Provider constructor - Initialize default requestUserAgent in provider constructors - Remove default UA values from method parameters - Update all provider implementations for consistent behavior - Update test cases to reflect new request header handling
…endencies - Move npm configuration from .npmrc to pnpm-workspace.yaml - Remove deprecated .npmrc file - Modernize package management configuration
- Migrate patchedDependencies to pnpm-workspace.yaml - Replace onlyBuiltDependencies/ignoredBuiltDependencies with newer allowBuilds format for granular control over package builds - Remove pnpm config block from package.json Improves configuration organization and uses modern pnpm settings format as recommended in pnpm v8+.
- Change build command from "tsc -p tsconfig.build.json" to "tsc --build tsconfig.build.json" - This update aligns with TypeScript's project reference feature for improved build performance and management.
Update the Provider cache key version number to invalidate previous cache entries and ensure compatibility with new request headers handling implementation.
…ource - Add debug logging for request URL, headers, and cache key - Log cache hit and miss scenarios for better observability - Wrap cache operation in try-catch block for consistent error handling - Clean up trailing whitespace Improves debugging and error tracking during provider resource requests.
Only use the HTTP agent in production environment. In development and testing environments, disable the agent to avoid connection pooling issues and allow more flexible network configuration.
✅ Deploy Preview for surgio-documentation ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #322 +/- ##
==========================================
+ Coverage 74.86% 75.04% +0.17%
==========================================
Files 66 66
Lines 2845 2957 +112
Branches 754 780 +26
==========================================
+ Hits 2130 2219 +89
- Misses 447 466 +19
- Partials 268 272 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Update documentation to use pnpm for running individual test files, which is consistent with other test commands in the project.
- Add test for invalid YAML syntax in getClashSubscription - Add test for filtering unsupported nodes in parseClashConfig - Add test for socks5 options handling - Add test for CustomProvider nodeList function receiving params - Add test for CustomProvider hook overriding node list - Add test for unknown node type validation - Add test for vmess compatibility rules These tests improve coverage for request header handling and edge cases in provider configuration parsing.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a more flexible header handling system for provider requests and modernizes the codebase with significant tooling upgrades. The main feature addition is the passGatewayRequestHeaders configuration that allows specific headers to be forwarded from gateway requests to upstream provider servers, replacing the more limited passGatewayRequestUserAgent option.
Changes:
- Introduces
passGatewayRequestHeadersconfiguration for flexible header forwarding with a whitelist mechanism - Adds unified
getNodeListV2method to all providers for efficient retrieval of both node lists and subscription user info in a single call - Replaces
agentkeepalivedependency with native Node.js HTTP agents - Migrates ESLint from v8 to v9 with flat config format, upgrading related dependencies
- Upgrades pnpm from v9 to v10 and updates numerous dependencies
- Refactors subscription user info property naming from
subscriptionUserinfotosubscriptionUserInfofor consistency
Reviewed changes
Copilot reviewed 42 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json, tsconfig.build.json | TypeScript configuration updates for composite builds |
| src/validators/surgio-config.ts | Adds validation for new passRequestHeaders configuration |
| src/utils/useragent.ts | Updates regex patterns to escape dots correctly |
| src/utils/subscription.ts | Improves null safety in subscription parsing |
| src/utils/http-client.ts | Replaces agentkeepalive with native Node.js agents |
| src/provider/Provider.ts | Adds header handling logic, cache key generation, and getNodeListV2 abstract method |
| src/provider/*.ts | Implements getNodeListV2 across all providers with consistent patterns |
| src/provider/tests/*.test.ts | Updates tests to include required requestHeaders and cacheKey parameters |
| src/generator/artifact.ts | Updates to use getNodeListV2 and store subscription user info |
| src/constant/constant.ts | Adds header whitelist and bumps provider cache key version |
| src/config.ts | Adds default configuration for passRequestHeaders |
| eslint.config.mjs | New flat config format for ESLint v9 |
| package.json | Major dependency updates including ESLint v9, pnpm v10, and various libraries |
| pnpm-workspace.yaml | Moves configuration from .npmrc to workspace config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/generator/artifact.ts
Outdated
| public providerMap: Map<string, PossibleProviderType> = new Map() | ||
| public nodeList: PossibleNodeConfigType[] = [] | ||
| public subscriptionUserInfo?: SubscriptionUserinfo | ||
| public subscriptionUserinfoMap: Map<string, SubscriptionUserinfo> = new Map() |
There was a problem hiding this comment.
The property name has been changed from subscriptionUserinfo to subscriptionUserInfo (capital 'I'). However, the old spelling is still used in one place - the Map name subscriptionUserinfoMap on line 80 should be updated to subscriptionUserInfoMap for consistency with the new naming convention.
Suggested change
| public subscriptionUserinfoMap: Map<string, SubscriptionUserinfo> = new Map() | |
| public subscriptionUserInfoMap: Map<string, SubscriptionUserinfo> = new Map() |
src/generator/artifact.ts
Outdated
|
|
||
| // Store subscriptionUserInfo for all providers in the map | ||
| if (subscriptionUserInfo) { | ||
| this.subscriptionUserinfoMap.set(providerName, subscriptionUserInfo) |
There was a problem hiding this comment.
The Map is declared with the old property name subscriptionUserinfoMap on line 80, but it's being used here with the new subscriptionUserInfo values. For consistency with the naming convention change throughout the codebase (subscriptionUserinfo → subscriptionUserInfo), this Map should be renamed to subscriptionUserInfoMap along with its usage on line 528.
src/utils/http-client.ts
Outdated
| 'user-agent': getUserAgent(), | ||
| }, | ||
| agent, | ||
| agent: process.env.NODE_ENV === 'production' ? agent : undefined, |
There was a problem hiding this comment.
The agent is disabled in non-production environments by setting it to undefined. This means connection pooling and keep-alive features will be disabled during development and testing, which could significantly impact performance and behavior differences between test and production environments. Consider whether this is intentional or if tests should also use the agent configuration.
Comment on lines
+56
to
+66
| if (getConfig()?.gateway?.passRequestUserAgent) { | ||
| if (!this.passGatewayRequestHeaders.includes('user-agent')) { | ||
| this.passGatewayRequestHeaders.push('user-agent') | ||
| } | ||
| } | ||
|
|
||
| for (const header of PASS_GATEWAY_REQUEST_HEADERS_WHITELIST) { | ||
| if (!this.passGatewayRequestHeaders.includes(header)) { | ||
| this.passGatewayRequestHeaders.push(header) | ||
| } | ||
| } |
There was a problem hiding this comment.
The whitelist headers are always added to passGatewayRequestHeaders regardless of whether they are already configured. While the duplicate check prevents adding the same header twice, the logic could be clearer. Since the whitelist already includes 'user-agent', the conditional check for passRequestUserAgent on lines 56-60 is somewhat redundant. Consider whether the whitelist should be applied after user configuration or if there should be a clear precedence order.
| * | ||
| * Providers must implement this to efficiently fetch both data when they come from the same source. | ||
| */ | ||
| abstract getNodeListV2: GetNodeListV2Function |
There was a problem hiding this comment.
The new getNodeListV2 method has been added to all provider implementations but there are no dedicated tests for this new method. While the existing code uses getNodeListV2 internally, there should be explicit test coverage to verify that the method correctly returns both nodeList and subscriptionUserInfo in a single call, especially for providers that support subscription user info (ClashProvider, TrojanProvider, SsdProvider, etc.).
Comment on lines
+89
to
+92
| passRequestHeaders: z | ||
| .array(z.string()) | ||
| .default([]) | ||
| .transform((val) => val.map((item) => item.toLowerCase())), |
There was a problem hiding this comment.
The new passRequestHeaders configuration feature is not documented. Users need documentation explaining how to configure this feature, what headers are allowed to pass through, and the relationship between passRequestHeaders and the legacy passRequestUserAgent option. Consider adding documentation in the appropriate location (e.g., README, docs folder, or CHANGELOG).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Updated the variable name from `subscriptionUserinfoMap` to `subscriptionUserInfoMap` for consistency and clarity in the Artifact class. This change improves code readability and aligns with naming conventions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
passGatewayRequestHeadersconfiguration to pass request headers through the gatewaypassGatewayRequestUserAgenttopassGatewayRequestHeadersfor more flexible header handlinggetNodeListV2methodagentkeepalivewith native Node.js HTTP agentsTest plan