ronantakizawa/codereview-bench huggingface dataset:
Repository: denoland/fresh
File: src/middlewares/csrf.ts (TypeScript)

Code before:
 *
 * @property origin - Specifies the allowed origin(s) for requests. Can be a string, an array of strings, or a custom handler function.
 *   - string: A single allowed origin.
 *   - string[]: Multiple allowed origins.
 *   - IsAllowedOriginHandler: A function to determine if an origin is allowed.
 */
export interface CsrfOptions {
  origin?: string | string[] | IsAllowedOriginHandler;
}

function isSafeMethod(method: string): boolean {
  return method === "GET" || method === "HEAD";
}

function isRequestedByFormElement(contentType: string): boolean {
  const contentTypeLower = contentType.toLowerCase();

  return contentTypeLower === "application/x-www-form-urlencoded" ||
    contentTypeLower === "multipart/form-data" ||
    contentTypeLower === "text/plain";
}

/**
 * CSRF Protection Middleware for Fresh.
 * Adapted from Hono's CORS middleware.
 * Would like to express my gratitude to all the developers of Hono.
 *
 * @param options Options for the CSRF protection middleware.
 * @returns The middleware handler function.
 *
 * @example Specifying origins with using `origin` option: default
 * ```ts
 * const app = new App<State>()
 *
 * app.use(csrf())
 * ```
 *
 * @example Specifying origins with using `origin` option: string
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
 * )
 * ```
 *
 * @example Specifying origins with using `origin` option: function
 * It is strongly recommended that the protocol be verified to ensure a match to `$`.

Review comment (suggestion):
```suggestion
```
I don't think a personal note is needed in documentation for a symbol.

Code after:
}

/**
 * CSRF Protection Middleware for Fresh.
 *
 * @param options Options for the CSRF protection middleware.
 * @returns The middleware handler function.
 *
 * @example Basic usage (with defaults)
 * ```ts
 * const app = new App<State>()
 *
 * app.use(csrf())
 * ```
 *
 * @example Specifying static origins
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
 * )
 * ```
 *
 * @example Specifying more complex origins
 * ```ts
 * app.use(
 *   '*',
 *   csrf({
 *     origin: (origin) => ['https://myapp.example.com', 'http://development.myapp.example.com'].includes(origin),
 *   })
 * )
 * ```
 */
export function csrf<State>(
  options?: CsrfOptions,
): Middleware<State> {
  const isAllowedOrigin = (
    origin: string | null,
    ctx: Context<State>,
  ) => {
    if (origin === null) {
      return false;
    }

    const optsOrigin = options?.origin;

    if (!optsOrigin) {
Repository: denoland/fresh
File: src/middlewares/csrf.ts (TypeScript)

Code before:
}

function isRequestedByFormElement(contentType: string): boolean {
  const contentTypeLower = contentType.toLowerCase();

  return contentTypeLower === "application/x-www-form-urlencoded" ||
    contentTypeLower === "multipart/form-data" ||
    contentTypeLower === "text/plain";
}

/**
 * CSRF Protection Middleware for Fresh.
 * Adapted from Hono's CORS middleware.
 * Would like to express my gratitude to all the developers of Hono.
 *
 * @param options Options for the CSRF protection middleware.
 * @returns The middleware handler function.
 *
 * @example Specifying origins with using `origin` option: default
 * ```ts
 * const app = new App<State>()
 *
 * app.use(csrf())
 * ```
 *
 * @example Specifying origins with using `origin` option: string
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
 * )
 * ```
 *
 * @example Specifying origins with using `origin` option: function
 * It is strongly recommended that the protocol be verified to ensure a match to `$`.
 * You should *never* do a forward match.
 *
 * ```ts
 * app.use(
 *   '*',
 *   csrf({
 *     origin: (origin) => /https:\/\/(\w+\.)?myapp\.example\.com$/.test(origin),
 *   })
 * )
 * ```
 */
export function csrf(

Review comment (suggestion):
```suggestion
 * @example Specifying static origins
```

Code after:
 * app.use(csrf())
 * ```
 *
 * @example Specifying static origins
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
 * )
 * ```
 *
 * @example Specifying more complex origins
 * ```ts
 * app.use(
 *   '*',
 *   csrf({
 *     origin: (origin) => ['https://myapp.example.com', 'http://development.myapp.example.com'].includes(origin),
 *   })
 * )
 * ```
 */
export function csrf<State>(
  options?: CsrfOptions,
): Middleware<State> {
  const isAllowedOrigin = (
    origin: string | null,
    ctx: Context<State>,
  ) => {
    if (origin === null) {
      return false;
    }

    const optsOrigin = options?.origin;

    if (!optsOrigin) {
      return origin === ctx.url.origin;
    }
    if (typeof optsOrigin === "string") {
      return origin === optsOrigin;
    }
    if (typeof optsOrigin === "function") {
      return optsOrigin(origin, ctx);
    }
    return Array.isArray(optsOrigin) && optsOrigin.includes(origin);
  };

  return async (ctx) => {
Repository: denoland/fresh
File: src/middlewares/csrf.ts (TypeScript)

Code before:
 *
 * @param options Options for the CSRF protection middleware.
 * @returns The middleware handler function.
 *
 * @example Specifying origins with using `origin` option: default
 * ```ts
 * const app = new App<State>()
 *
 * app.use(csrf())
 * ```
 *
 * @example Specifying origins with using `origin` option: string
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
 * )
 * ```
 *
 * @example Specifying origins with using `origin` option: function
 * It is strongly recommended that the protocol be verified to ensure a match to `$`.
 * You should *never* do a forward match.
 *
 * ```ts
 * app.use(
 *   '*',
 *   csrf({
 *     origin: (origin) => /https:\/\/(\w+\.)?myapp\.example\.com$/.test(origin),
 *   })
 * )
 * ```
 */
export function csrf(
  options?: CsrfOptions,
): (ctx: FreshContext) => Promise<Response> {
  const isAllowedOrigin = (
    origin: string | undefined | null,
    ctx: FreshContext,
  ) => {
    if (!origin) {
      return false;
    }

    const optsOrigin = options?.origin;

    if (!optsOrigin) {
      return origin === ctx.url.origin;

Review comment (suggestion):
```suggestion
 * @example Specifying more complex origins
```
I think any explanation should be clear, concise and lie between the example title and the snippet.

Code after:
 *
 * @example Specifying more complex origins
 * ```ts
 * app.use(
 *   '*',
 *   csrf({
 *     origin: (origin) => ['https://myapp.example.com', 'http://development.myapp.example.com'].includes(origin),
 *   })
 * )
 * ```
 */
export function csrf<State>(
  options?: CsrfOptions,
): Middleware<State> {
  const isAllowedOrigin = (
    origin: string | null,
    ctx: Context<State>,
  ) => {
    if (origin === null) {
      return false;
    }

    const optsOrigin = options?.origin;

    if (!optsOrigin) {
      return origin === ctx.url.origin;
    }
    if (typeof optsOrigin === "string") {
      return origin === optsOrigin;
    }
    if (typeof optsOrigin === "function") {
      return optsOrigin(origin, ctx);
    }
    return Array.isArray(optsOrigin) && optsOrigin.includes(origin);
  };

  return async (ctx) => {
    const { method, headers } = ctx.req;

    // Safe methods
    if (method === "GET" || method === "HEAD" || method === "OPTIONS") {
      return await ctx.next();
    }

    const secFetchSite = headers.get("Sec-Fetch-Site");
    const origin = headers.get("origin");

    if (secFetchSite !== null) {
      if (
        secFetchSite === "same-origin" || secFetchSite === "none" ||
        isAllowedOrigin(origin, ctx)
Repository: denoland/fresh
File: src/middlewares/csrf_test.ts (TypeScript)

Code before:
import { App } from "../app.ts";
import type { FreshContext } from "../context.ts";
import { csrf } from "./csrf.ts";
import { expect } from "@std/expect";
import { spy } from "@std/testing/mock";

const rawSimplePostHandler = async (ctx: FreshContext) => {
  if (ctx.req.headers.get("content-type") === "application/json") {
    return new Response((await ctx.req.json())["name"]);
  } else {
    return new Response((await ctx.req.formData()).get("name"));
  }
};

Deno.test("CSRF by Middleware", async (t) => {
  let simplePostHandler = spy(rawSimplePostHandler);

  //beforeEach(() => {
  //  console.log("Resetting simplePostHandler spy");
  //  simplePostHandler = spy(rawSimplePostHandler);
  //});

  await t.step("simple usage", async (t) => {
    const app = new App();

    app.all("*", csrf());
    app.get("/form", () => new Response("<form></form>"));
    app.post("/form", simplePostHandler);
    app.put("/form", () => new Response("OK"));
    app.delete("/form", () => new Response("OK"));
    app.patch("/form", () => new Response("OK"));
    app.head("/form", () => new Response("OK"));

    const handler = app.handler();

    await t.step("GET /form - should be 200 for any request", async () => {
      simplePostHandler = spy(rawSimplePostHandler);

Review comment (suggestion):
```suggestion
  return ctx.req.headers.get("content-type") === "application/json"
    ? new Response((await ctx.req.json())["name"])
    : new Response((await ctx.req.formData()).get("name"));
```

Code after:
import { App } from "../app.ts";
import type { Method } from "../router.ts";
import { csrf, type CsrfOptions } from "./csrf.ts";
import { expect } from "@std/expect";

function testHandler<T>(
  options?: CsrfOptions<T>,
): (request: Request) => Promise<Response> {
  return new App<T>()
    .use(csrf(options))
    .all("/", () => new Response("hello"))
    .handler();
}

function createTests(trusted: CsrfOptions["origin"]) {
  return async function runTest(
    method: Method,
    expected: 200 | 403,
    test: {
      secFetchSite?: "cross-site" | "same-origin" | "same-site" | "none";
      origin?: string;
    } = {},
  ) {
    const handler = testHandler({ origin: trusted });

    const headers = new Headers();
    if (test.secFetchSite) headers.append("Sec-Fetch-Site", test.secFetchSite);
    if (test.origin) {
      headers.append("Origin", test.origin);
    }

    const res = await handler(
      new Request("https://example.com", { method, headers }),
    );
    await res.body?.cancel();
    expect(res.status).toEqual(expected);
  };
Repository: denoland/fresh
File: src/middlewares/csrf.ts (TypeScript)

Code before:
import type { FreshContext } from "../context.ts";
import { HttpError } from "../error.ts";

/**
 * Determines whether a given origin is allowed for CSRF protection.
 *
 * @param origin - The origin of the incoming request.
 * @param context - The FreshContext object for the current request.
 * @returns A boolean indicating if the origin is allowed.
 */
export type IsAllowedOriginHandler = (
  origin: string,
  context: FreshContext,
) => boolean;

/** Options for configuring the CSRF protection middleware. **/
export interface CsrfOptions {
  /**
   * origin - Specifies the allowed origin(s) for requests.
   *  - string: A single allowed origin.
   *  - string[]: static allowed origins.
   *  - IsAllowedOriginHandler: A function to determine if an origin is allowed.
   */
  origin?: string | string[] | IsAllowedOriginHandler;
}

function isSafeMethod(method: string): boolean {
  return method === "GET" || method === "HEAD";
}

function isRequestedByFormElement(contentType: string): boolean {
  const contentTypeLower = contentType.toLowerCase();

  return contentTypeLower === "application/x-www-form-urlencoded" ||
    contentTypeLower === "multipart/form-data" ||
    contentTypeLower === "text/plain";
}

/**
 * CSRF Protection Middleware for Fresh.
 *

Review comment (security):
```suggestion
/** Options for {@linkcode csrf}. **/
```

Code after:
import type { Context } from "../context.ts";
import { HttpError } from "../error.ts";
import type { Middleware } from "./mod.ts";

/** Options for {@linkcode csrf}. **/
// deno-lint-ignore no-explicit-any
export interface CsrfOptions<State = any> {
  /**
   * origin - Specifies the allowed origin(s) for requests.
   *  - string: A single allowed origin.
   *  - string[]: static allowed origins.
   *  - function: A function to determine if an origin is allowed.
   */
  origin?:
    | string
    | string[]
    | ((origin: string, context: Context<State>) => boolean);
}

/**
 * CSRF Protection Middleware for Fresh.
 *
 * @param options Options for the CSRF protection middleware.
 * @returns The middleware handler function.
 *
 * @example Basic usage (with defaults)
 * ```ts
 * const app = new App<State>()
 *
 * app.use(csrf())
 * ```
 *
 * @example Specifying static origins
 * ```ts
 * app.use(csrf({ origin: 'https://myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['https://myapp.example.com', 'http://development.myapp.example.com'],
 *   })
Repository: denoland/fresh
File: src/middlewares/csrf_test.ts (TypeScript)

Code before:
import { App } from "../app.ts";
import type { FreshContext } from "../context.ts";
import { csrf } from "./csrf.ts";
import { expect } from "@std/expect";
import { spy } from "@std/testing/mock";

const rawSimplePostHandler = async (ctx: FreshContext) => {
  if (ctx.req.headers.get("content-type") === "application/json") {
    return new Response((await ctx.req.json())["name"]);
  } else {
    return new Response((await ctx.req.formData()).get("name"));
  }
};

Deno.test("CSRF by Middleware", async (t) => {
  let simplePostHandler = spy(rawSimplePostHandler);

  //beforeEach(() => {
  //  console.log("Resetting simplePostHandler spy");
  //  simplePostHandler = spy(rawSimplePostHandler);
  //});

  await t.step("simple usage", async (t) => {
    const app = new App();

    app.all("*", csrf());
    app.get("/form", () => new Response("<form></form>"));
    app.post("/form", simplePostHandler);
    app.put("/form", () => new Response("OK"));
    app.delete("/form", () => new Response("OK"));
    app.patch("/form", () => new Response("OK"));
    app.head("/form", () => new Response("OK"));

    const handler = app.handler();

    await t.step("GET /form - should be 200 for any request", async () => {
      simplePostHandler = spy(rawSimplePostHandler);
      const res = await handler(new Request("http://localhost/form"));

      expect(res.status).toBe(200);
      expect(await res.text()).toBe("<form></form>");

Review comment (suggestion):
```suggestion
  using simplePostHandler = spy(rawSimplePostHandler);
```
Might not better but maybe best to do.

Code after:
import { App } from "../app.ts";
import type { Method } from "../router.ts";
import { csrf, type CsrfOptions } from "./csrf.ts";
import { expect } from "@std/expect";

function testHandler<T>(
  options?: CsrfOptions<T>,
): (request: Request) => Promise<Response> {
  return new App<T>()
    .use(csrf(options))
    .all("/", () => new Response("hello"))
    .handler();
}

function createTests(trusted: CsrfOptions["origin"]) {
  return async function runTest(
    method: Method,
    expected: 200 | 403,
    test: {
      secFetchSite?: "cross-site" | "same-origin" | "same-site" | "none";
      origin?: string;
    } = {},
  ) {
    const handler = testHandler({ origin: trusted });

    const headers = new Headers();
    if (test.secFetchSite) headers.append("Sec-Fetch-Site", test.secFetchSite);
    if (test.origin) {
      headers.append("Origin", test.origin);
    }

    const res = await handler(
      new Request("https://example.com", { method, headers }),
    );
    await res.body?.cancel();
    expect(res.status).toEqual(expected);
  };
}

Deno.test("CSRF - allow GET/HEAD/OPTIONS", async () => {
  const runTest = createTests("https://example.com");
Repository: denoland/fresh
File: www/utils/screenshot.ts (TypeScript)

Code before:
  id: string,
): { image2x: string; image1x: string } {
  return {
    image2x: `./www/static/showcase/${id}2x.jpg`,
    image1x: `./www/static/showcase/${id}1x.jpg`,
  };
}

export async function captureScreenshot(
  url: string,
  id: string,
): Promise<void> {
  const browser = await launch();
  try {
    const page = await browser.newPage(url);
    await page.waitForNetworkIdle();
    const raw = await page.screenshot();

    const image2x = await Image.decode(raw);
    const { image2x: path2x, image1x: path1x } = generateFilePaths(id);

    await Deno.writeFile(path2x, await image2x.encodeJPEG(80));
    const image1x = image2x.resize(image2x.width / 2, Image.RESIZE_AUTO);
    await Deno.writeFile(path1x, await image1x.encodeJPEG(80));
  } finally {
    await browser.close();
  }
}

// only run the script if it's the main module
if (import.meta.main) {
  try {
    const [url, id] = validateArgs(Deno.args);
    validateUrl(url);
    await captureScreenshot(url, id);
    // deno-lint-ignore no-console
    console.log(`Screenshot saved as ${id}1x.jpg and ${id}2x.jpg`);
  } catch (error) {
    const message = error instanceof Error ? error.message : String(error);
    // deno-lint-ignore no-console
    console.log(`Error: ${message}`);
    Deno.exit(1);
  }
}


Review comment (bug):
Is this line and the `try`/`catch` blocks needed? Does the browser close even if the script fails anyway?

Code after:
  id: string,
): { image2x: string; image1x: string } {
  return {
    image2x: `./www/static/showcase/${id}2x.jpg`,
    image1x: `./www/static/showcase/${id}1x.jpg`,
  };
}

export async function captureScreenshot(
  url: string,
  id: string,
): Promise<void> {
  const browser = await launch();
  try {
    const page = await browser.newPage(url);
    await page.waitForNetworkIdle();
    const raw = await page.screenshot();

    const image2x = await Image.decode(raw);
    const { image2x: path2x, image1x: path1x } = generateFilePaths(id);

    await Deno.writeFile(path2x, await image2x.encodeJPEG(80));
    const image1x = image2x.resize(image2x.width / 2, Image.RESIZE_AUTO);
    await Deno.writeFile(path1x, await image1x.encodeJPEG(80));
  } finally {
    await browser.close();
  }
}

// only run the script if it's the main module
if (import.meta.main) {
  const [url, id] = validateArgs(Deno.args);
  validateUrl(url);
  await captureScreenshot(url, id);
  // deno-lint-ignore no-console
  console.log(`Screenshot saved as ${id}1x.jpg and ${id}2x.jpg`);
}
Repository: denoland/fresh
File: src/runtime/client/partials.ts (TypeScript)

Code before:
      }

      const partialUrl = new URL(
        partial,
        location.href,
      );
      await fetchPartials(partialUrl, partialUrl);
    }
  }
});

addEventListener("popstate", async (e) => {
  // When state is `null` then the browser navigated to a document
  // fragment. In this case we do nothing.
  if (e.state === null) {
    // Reset to browser default
    if (history.scrollRestoration) {
      history.scrollRestoration = "auto";
    }
    return;
  }
  // Do nothing if Fresh navigation is not explicitly opted-in.
  // Other applications might manage scrollRestoration individually.
  if (e.state.fClientNav !== true) {
    return;
  }

  const state: FreshHistoryState = history.state;
  const nextIdx = state.index ?? index + 1;
  index = nextIdx;

  const setting = document.querySelector(`[${CLIENT_NAV_ATTR}]`);
  if (setting === null || setting.getAttribute(CLIENT_NAV_ATTR) === "false") {
    location.reload();
    return;
  }

  // We need to keep track of that ourselves since we do client side
  // navigation.
  if (history.scrollRestoration) {
    history.scrollRestoration = "manual";
  }

  const url = new URL(location.href, location.origin);
  try {
    await fetchPartials(url, url);
    updateLinks(url);
    scrollTo({
      left: state.scrollX ?? 0,
      top: state.scrollY ?? 0,
      behavior: "instant",

Review comment (suggestion):
```suggestion
  if (!e.state.fClientNav) return;
```

Code after:
      // Check if the user opted out of client side navigation.
      if (partial === null || !checkClientNavEnabled(el)) {
        return;
      }

      const partialUrl = new URL(
        partial,
        location.href,
      );
      await fetchPartials(partialUrl, partialUrl);
    }
  }
});

addEventListener("popstate", async (e) => {
  // When state is `null` then the browser navigated to a document
  // fragment. In this case we do nothing.
  if (e.state === null) {
    // Reset to browser default
    if (history.scrollRestoration) {
      history.scrollRestoration = "auto";
    }
    return;
  }
  // Do nothing if Fresh navigation is not explicitly opted-in.
  // Other applications might manage scrollRestoration individually.
  if (!e.state.fClientNav) return;

  const state: FreshHistoryState = history.state;
  const nextIdx = state.index ?? index + 1;
  index = nextIdx;

  const setting = document.querySelector(`[${CLIENT_NAV_ATTR}]`);
  if (setting === null || setting.getAttribute(CLIENT_NAV_ATTR) === "false") {
    location.reload();
    return;
  }

  // We need to keep track of that ourselves since we do client side
  // navigation.
  if (history.scrollRestoration) {
    history.scrollRestoration = "manual";
  }

  const url = new URL(location.href, location.origin);
  try {
    await fetchPartials(url, url);
    updateLinks(url);
    scrollTo({
      left: state.scrollX ?? 0,
      top: state.scrollY ?? 0,