feat: add active probing to HTTPGatewayRouter #830
Conversation
2color
changed the title
2color
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds active CORS probing functionality to the HTTPGatewayRouter to ensure only CORS-compatible gateways are returned as providers. The implementation includes caching of probe results to avoid repeated checks.
Key changes:
- Introduces CORS probing with configurable cache duration
- Transforms gateway storage from simple PeerInfo array to GatewayStatus objects with metadata
- Filters providers based on CORS compatibility before yielding them
| }) | ||
|
|
||
| const corsHeaders = response.headers.get('access-control-allow-origin') | ||
| const hasCors = corsHeaders === '*' || corsHeaders?.includes(window.location.origin) |
There was a problem hiding this comment.
The code assumes it's running in a browser environment by accessing window.location.origin, but this router may be used in Node.js environments where window is undefined. This will cause a runtime error in non-browser environments.
| const hasCors = corsHeaders === '*' || corsHeaders?.includes(window.location.origin) | |
| const origin = (typeof window !== 'undefined' && window.location?.origin) || '' | |
| const hasCors = corsHeaders === '*' || corsHeaders?.includes(origin) |
Copilot uses AI. Check for mistakes.
| return gateway.corsSupported | ||
| } | ||
|
|
||
| const gatewayUrl = gateway.peerInfo.multiaddrs[0]?.toString().replace('/http', 'http').replace('/https', 'https') |
There was a problem hiding this comment.
The string replacement logic is fragile and could produce incorrect URLs. For example, a multiaddr containing '/http' in the middle would be incorrectly transformed. Consider using proper multiaddr parsing or URL construction methods.
| const gatewayUrl = gateway.peerInfo.multiaddrs[0]?.toString().replace('/http', 'http').replace('/https', 'https') | |
| const multiaddr = gateway.peerInfo.multiaddrs[0] | |
| const gatewayUrl = multiaddr ? uriToMultiaddr(multiaddr).toString() : null |
Copilot uses AI. Check for mistakes.
|
|
||
| private async checkCorsSupport (gatewayUrl: string): Promise<boolean> { | ||
| try { | ||
| const response = await fetch(`${gatewayUrl}/ipfs/bafkqaaa`, { |
There was a problem hiding this comment.
The hardcoded CID 'bafkqaaa' should be defined as a named constant to improve maintainability and make its purpose clear (appears to be an empty file CID for testing).
| const response = await fetch(`${gatewayUrl}/ipfs/bafkqaaa`, { | |
| const response = await fetch(`${gatewayUrl}/ipfs/${EMPTY_FILE_CID}`, { |
Copilot uses AI. Check for mistakes.
|
|
||
| const corsHeaders = response.headers.get('access-control-allow-origin') | ||
| const hasCors = corsHeaders === '*' || corsHeaders?.includes(window.location.origin) | ||
|
|
There was a problem hiding this comment.
The logic for accepting 404 status as a valid response should be documented. It's not immediately clear why a 404 response indicates CORS support.
| // Some HTTP gateways may return a 404 status for non-existent resources while still supporting CORS. | |
| // The presence of CORS headers is the primary determinant of CORS support, and a 404 status is treated | |
| // as valid in this context to account for such behavior. |
Copilot uses AI. Check for mistakes.
Background
Arkadiy from the Internet Archive mentioned he wanted more control over the default HTTPGatewayRouter.
This demonstrates how you would add active probing to ensure CORS.
Description
This PR adds active CORS probing functionality to the HTTPGatewayRouter to ensure only CORS-compatible gateways are returned as providers. The implementation includes caching of probe results to avoid repeated checks.
Key changes: