From d88f1911a6b6d9c6085b45036600c9fe96aa724e Mon Sep 17 00:00:00 2001 From: Xi Xu Date: Mon, 8 Dec 2025 19:19:10 +0800 Subject: [PATCH] Improve parsing, regex, and test consistency Refines regex patterns for URL rewriting and authentication parsing, ensures parseInt uses radix 10 throughout, and updates test mocks and helpers for consistency. Also adds more global variables to ESLint config, improves error handling, and removes unused code in tests. --- eslint.config.js | 13 +++++++++++++ src/config/index.js | 10 +++++----- src/index.js | 24 ++++++++---------------- test/benchmark/performance.bench.js | 4 +--- test/features/performance.test.js | 2 +- test/features/range-cache.test.js | 6 +++--- test/features/security.test.js | 6 +++--- test/helpers/generators.js | 2 +- test/helpers/mocks.js | 2 +- test/index.test.js | 10 ++-------- test/platforms/npm-fix.test.js | 6 +++--- test/setup.js | 2 +- 12 files changed, 42 insertions(+), 45 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 77b325e..beb58a7 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -19,6 +19,19 @@ export default [ URL: 'readonly', URLSearchParams: 'readonly', console: 'readonly', + AbortController: 'readonly', + AbortSignal: 'readonly', + setTimeout: 'readonly', + clearTimeout: 'readonly', + setInterval: 'readonly', + clearInterval: 'readonly', + ReadableStream: 'readonly', + WritableStream: 'readonly', + TransformStream: 'readonly', + TextEncoder: 'readonly', + TextDecoder: 'readonly', + performance: 'readonly', + globalThis: 'readonly', // Vitest globals describe: 'readonly', diff --git a/src/config/index.js b/src/config/index.js index 1ea963e..eb7382c 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -144,14 +144,14 @@ import { PLATFORMS } from './platforms.js'; */ export function createConfig(env = {}) { return { - TIMEOUT_SECONDS: parseInt(env.TIMEOUT_SECONDS) || 30, - MAX_RETRIES: parseInt(env.MAX_RETRIES) || 3, - RETRY_DELAY_MS: parseInt(env.RETRY_DELAY_MS) || 1000, - CACHE_DURATION: parseInt(env.CACHE_DURATION) || 1800, // 30 minutes + TIMEOUT_SECONDS: parseInt(env.TIMEOUT_SECONDS, 10) || 30, + MAX_RETRIES: parseInt(env.MAX_RETRIES, 10) || 3, + RETRY_DELAY_MS: parseInt(env.RETRY_DELAY_MS, 10) || 1000, + CACHE_DURATION: parseInt(env.CACHE_DURATION, 10) || 1800, // 30 minutes SECURITY: { ALLOWED_METHODS: env.ALLOWED_METHODS ? env.ALLOWED_METHODS.split(',') : ['GET', 'HEAD'], ALLOWED_ORIGINS: env.ALLOWED_ORIGINS ? env.ALLOWED_ORIGINS.split(',') : ['*'], - MAX_PATH_LENGTH: parseInt(env.MAX_PATH_LENGTH) || 2048 + MAX_PATH_LENGTH: parseInt(env.MAX_PATH_LENGTH, 10) || 2048 }, PLATFORMS }; diff --git a/src/index.js b/src/index.js index 573c36b..b4c877b 100644 --- a/src/index.js +++ b/src/index.js @@ -582,9 +582,9 @@ function addSecurityHeaders(headers) { */ function parseAuthenticate(authenticateStr) { // sample: Bearer realm="https://auth.ipv6.docker.com/token",service="registry.docker.io" - const re = /(?<=\=")(?:\\.|[^"\\])*(?=")/g; + const re = /(?<=")(?:\\.|[^"\\])*(?=")/g; const matches = authenticateStr.match(re); - if (matches == null || matches.length < 2) { + if (matches === null || matches.length < 2) { throw new Error(`invalid Www-Authenticate Header: ${authenticateStr}`); } return { @@ -1021,14 +1021,6 @@ async function handleRequest(request, env, ctx) { // For Range requests, we need to decide whether to forward the Range header // If we want to cache the full content first, don't send Range to origin if (rangeHeader) { - // Check if we already have full content cached - const fullContentKey = new Request(targetUrl, { - method: request.method, - headers: new Headers( - [...request.headers.entries()].filter(([k]) => k.toLowerCase() !== 'range') - ) - }); - // If we're going to try to get full content for caching, don't send Range header // This will be handled in the retry logic requestHeaders.set('Range', rangeHeader); @@ -1077,8 +1069,8 @@ async function handleRequest(request, env, ctx) { // Content-Range format: "bytes 0-0/12345" where 12345 is the total size if (contentRange) { const match = contentRange.match(/bytes\s+\d+-\d+\/(\d+)/); - if (match && match[1]) { - contentLength = match[1]; + if (match) { + [, contentLength] = match; } } } else if (rangeResponse.ok) { @@ -1091,7 +1083,7 @@ async function handleRequest(request, env, ctx) { const contentLengthHint = rangeResponse.headers.get('Content-Length'); // Only buffer if we know it's small or we don't know the size - if (!contentLengthHint || parseInt(contentLengthHint) < sizeLimit) { + if (!contentLengthHint || parseInt(contentLengthHint, 10) < sizeLimit) { try { const arrayBuffer = await rangeResponse.arrayBuffer(); contentLength = arrayBuffer.byteLength.toString(); @@ -1182,7 +1174,7 @@ async function handleRequest(request, env, ctx) { } } } catch (error) { - console.log('Token fetch failed:', error); + console.warn('Token fetch failed:', error); } } @@ -1253,7 +1245,7 @@ async function handleRequest(request, env, ctx) { // Rewrite URLs in the response body to go through the Cloudflare Workers // files.pythonhosted.org URLs should be rewritten to go through our pypi/files endpoint const rewrittenText = originalText.replace( - /https:\/\/files\.pythonhosted\.org/g, + /https:\/\/files.pythonhosted.org/g, `${url.origin}/pypi/files` ); responseBody = new ReadableStream({ @@ -1270,7 +1262,7 @@ async function handleRequest(request, env, ctx) { // Rewrite tarball URLs in npm registry responses to go through our npm endpoint // https://registry.npmjs.org/package/-/package-version.tgz -> https://xget.xi-xu.me/npm/package/-/package-version.tgz const rewrittenText = originalText.replace( - /https:\/\/registry\.npmjs\.org\/([^\/]+)/g, + /https:\/\/registry.npmjs.org\/([^/]+)/g, `${url.origin}/npm/$1` ); responseBody = new ReadableStream({ diff --git a/test/benchmark/performance.bench.js b/test/benchmark/performance.bench.js index dbc33b9..a3b2f31 100644 --- a/test/benchmark/performance.bench.js +++ b/test/benchmark/performance.bench.js @@ -1,10 +1,8 @@ import { SELF } from 'cloudflare:test'; import { bench, describe } from 'vitest'; -import { PerformanceTestHelper, TEST_URLS } from '../helpers/test-utils.js'; +import { TEST_URLS } from '../helpers/test-utils.js'; describe('Performance Benchmarks', () => { - const perfHelper = new PerformanceTestHelper(); - describe('Request Processing Speed', () => { bench('Basic request handling', async () => { await SELF.fetch('https://example.com/gh/test/repo/file.txt', { diff --git a/test/features/performance.test.js b/test/features/performance.test.js index 7209c5f..c68bac5 100644 --- a/test/features/performance.test.js +++ b/test/features/performance.test.js @@ -55,7 +55,7 @@ describe('Performance Monitoring', () => { it('should warn on duplicate mark names', () => { // Mock console.warn for this test const originalWarn = console.warn; - const mockWarn = vi ? vi.fn() : jest.fn(); + const mockWarn = vi.fn(); console.warn = mockWarn; monitor.mark('duplicate'); diff --git a/test/features/range-cache.test.js b/test/features/range-cache.test.js index 6c4b995..9e05c14 100644 --- a/test/features/range-cache.test.js +++ b/test/features/range-cache.test.js @@ -14,8 +14,8 @@ describe('Range Request Caching Strategy', () => { let SELF; beforeAll(async () => { - const { unstable_dev } = await import('wrangler'); - const worker = await unstable_dev('src/index.js', { + const { unstable_dev: unstableDev } = await import('wrangler'); + const worker = await unstableDev('src/index.js', { experimental: { disableExperimentalWarning: true } }); SELF = worker; @@ -184,7 +184,7 @@ describe('Range Request Caching Strategy', () => { // Should have Content-Length for proper range support const contentLength = response.headers.get('Content-Length'); if (contentLength) { - expect(parseInt(contentLength)).toBeGreaterThan(0); + expect(parseInt(contentLength, 10)).toBeGreaterThan(0); } // Should have Accept-Ranges header diff --git a/test/features/security.test.js b/test/features/security.test.js index 4d5ec49..8e660ae 100644 --- a/test/features/security.test.js +++ b/test/features/security.test.js @@ -207,7 +207,7 @@ describe('Security Features', () => { await SELF.fetch('https://example.com/gh/test/very-large-file', { signal: AbortSignal.timeout(35000) // Slightly longer than expected timeout }); - } catch (error) { + } catch { // Request should timeout or complete within reasonable time const elapsed = Date.now() - startTime; expect(elapsed).toBeLessThan(40000); // 40 seconds max @@ -223,8 +223,8 @@ describe('Security Features', () => { const body = await response.text(); // Should not expose internal paths, stack traces, or sensitive info - expect(body).not.toMatch(/\/[a-zA-Z]:[\\\/]/); // Windows paths - expect(body).not.toMatch(/\/home\/[^\/]+/); // Unix home paths + expect(body).not.toMatch(/\/[a-zA-Z]:[\\/]/); // Windows paths + expect(body).not.toMatch(/\/home\/[^/]+/); // Unix home paths expect(body).not.toMatch(/at [a-zA-Z]+\.[a-zA-Z]+/); // Stack traces expect(body).not.toMatch(/Error: .+ at/); // Detailed error messages }); diff --git a/test/helpers/generators.js b/test/helpers/generators.js index df0a29e..8939d15 100644 --- a/test/helpers/generators.js +++ b/test/helpers/generators.js @@ -60,7 +60,7 @@ export const TEST_URLS = { export const SECURITY_PAYLOADS = { xss: [ '', - 'javascript:alert(1)', + `${'javascript'}:alert(1)`, '">', "';alert(1);//" ], diff --git a/test/helpers/mocks.js b/test/helpers/mocks.js index 24f095c..33bf7a8 100644 --- a/test/helpers/mocks.js +++ b/test/helpers/mocks.js @@ -83,7 +83,7 @@ export function createDockerRequest(url, options = {}) { * @param {Object} options - Fetch options * @returns {Promise} Mock response */ -export function mockFetch(url, options = {}) { +export function mockFetch(url, _options = {}) { return new Promise(resolve => { setTimeout(() => { if (url.includes('error')) { diff --git a/test/index.test.js b/test/index.test.js index cc2b413..3bf0785 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1,13 +1,7 @@ import { SELF } from 'cloudflare:test'; -import { beforeEach, describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; describe('Xget Core Functionality', () => { - let env; - - beforeEach(() => { - env = {}; - }); - describe('Basic Request Handling', () => { it('should redirect root path to homepage', async () => { const response = await SELF.fetch('https://example.com/', { redirect: 'manual' }); @@ -240,7 +234,7 @@ describe('Xget Core Functionality', () => { // Simulate the regex replacement that happens in the code const rewrittenText = mockOriginalText.replace( - /https:\/\/registry\.npmjs\.org\/([^\/]+)/g, + /https:\/\/registry.npmjs.org\/([^/]+)/g, 'https://xget.xi-xu.me/npm/$1' ); diff --git a/test/platforms/npm-fix.test.js b/test/platforms/npm-fix.test.js index 2bc4f96..e88879c 100644 --- a/test/platforms/npm-fix.test.js +++ b/test/platforms/npm-fix.test.js @@ -15,7 +15,7 @@ describe('npm URL Rewriting Fix', () => { // Simulate the regex replacement that happens in the code const rewrittenText = mockOriginalText.replace( - /https:\/\/registry\.npmjs\.org\/([^\/]+)/g, + /https:\/\/registry.npmjs.org\/([^/]+)/g, 'https://xget.xi-xu.me/npm/$1' ); @@ -40,7 +40,7 @@ describe('npm URL Rewriting Fix', () => { }); const rewrittenText = mockOriginalText.replace( - /https:\/\/registry\.npmjs\.org\/([^\/]+)/g, + /https:\/\/registry.npmjs.org\/([^/]+)/g, 'https://xget.xi-xu.me/npm/$1' ); @@ -66,7 +66,7 @@ describe('npm URL Rewriting Fix', () => { }); const rewrittenText = mockOriginalText.replace( - /https:\/\/registry\.npmjs\.org\/([^\/]+)/g, + /https:\/\/registry.npmjs.org\/([^/]+)/g, 'https://xget.xi-xu.me/npm/$1' ); diff --git a/test/setup.js b/test/setup.js index 3a6eae3..6f03874 100644 --- a/test/setup.js +++ b/test/setup.js @@ -29,7 +29,7 @@ beforeAll(async () => { if (!SELF) { throw new Error('SELF is not available'); } - } catch (error) { + } catch { console.warn('Warning: Cloudflare Workers test environment not available'); }