diff --git a/src/worker.ts b/src/worker.ts index 0309590..09f0ba6 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -9,20 +9,24 @@ import { } from './util'; import responses from './responses'; -export default { - async fetch( - request: Request, - env: Env, - ctx: ExecutionContext - ): Promise { +interface Worker { + async fetch: (r: Request, e: Env, c: ExecutionContext) => Promise; +} + +const cloudflareWorker: Worker = { + async fetch = (request, env, ctx) => { const cache = caches.default; - if (request.method !== 'GET' && request.method !== 'HEAD') { + + const shouldServeCache = isCacheEnabled(env); + + // Review: What about `OPTIONS` type? + if (['GET', 'HEAD'].includes(request.method) === false) { // This endpoint is called from the sync script to purge // directories that are commonly updated so we don't need to // wait for the cache to expire if ( - isCacheEnabled(env) && - env.PURGE_API_KEY !== undefined && + shouldServeCache && // <-- simplify this big if condition either by separating this condition to variables or smaller parts that are easier to read + env.PURGE_API_KEY !== undefined && // <-- shouldn't this always be defined? what's the point of verifying this env var? Maybe this check should be inside cachePurgeHandler :) request.method === 'POST' && request.url === '/_cf/cache-purge' ) { @@ -32,46 +36,52 @@ export default { return responses.METHOD_NOT_ALLOWED; } - if (isCacheEnabled(env)) { + if (shouldServeCache) { // Caching is enabled, let's see if the request is cached const response = await cache.match(request); - if (response !== undefined) { - console.log('Cache hit'); + + if (typeof response !== 'undefined') { response.headers.append('x-cache-status', 'hit'); + return response; } - - console.log('Cache miss'); } - const url = new URL(request.url); - let bucketPath = mapUrlPathToBucketPath(url, env); - if (bucketPath === undefined) { + const url = new URL(request.url); // <--- you should catch if URL is malformed. new URL can throw an Exception + + const bucketPath = mapUrlPathToBucketPath(url, env); + + if (typeof bucketPath === 'undefined') { // Directory listing is restricted and we're not on // a supported path, block request return new Response('Unauthorized', { status: 401 }); } - bucketPath = decodeURIComponent(bucketPath); - let response: Response; - if (isDirectoryPath(bucketPath)) { + // Review: Not a good practice to assign a variable to two completely different things + const bucketComponents = decodeURIComponent(bucketPath); + const isPathADirectory = isDirectoryPath(bucketPath); + + if (isPathADirectory && env.DIRECTORY_LISTING === 'off') { + // File not found since we should only be allowing + // file paths if directory listing is off + return responses.FILE_NOT_FOUND(request); + } + + const response: Response = isPathADirectory ? // Directory requested, try listing it - if (env.DIRECTORY_LISTING === 'off') { - // File not found since we should only be allowing - // file paths if directory listing is off - return responses.FILE_NOT_FOUND(request); - } - response = await directoryHandler(url, request, bucketPath, env); - } else { + await directoryHandler(url, request, bucketComponents, env) : // File requested, try to serve it - response = await fileHandler(url, request, bucketPath, env); - } + await fileHandler(url, request, bucketComponents, env); // Cache response if cache is enabled - if (isCacheEnabled(env) && response.status !== 304) { + if (shouldServeCache && response.status !== 304) { ctx.waitUntil(cache.put(request, response.clone())); } + response.headers.append('x-cache-status', 'miss'); + return response; }, }; + +export default cloudflareWorker;