Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request Body can be cumbersome #5763

Closed
ThePrimeagen opened this issue May 22, 2020 · 8 comments
Closed

Request Body can be cumbersome #5763

ThePrimeagen opened this issue May 22, 2020 · 8 comments

Comments

@ThePrimeagen
Copy link

Could I suggest a convenience method for reading all the data out of a request body? If the request is a chunky request or does not contain content-length you have to write this code which seems to be better a utility. Here is an example.

import {
    serve
} from "https://deno.land/std/http/server.ts";

import { getEnv } from "./env.ts";
import { config } from "https://deno.land/x/dotenv/mod.ts";

const c = config({
    export: true
});

const s = serve({
    port: 8002
});

for await (const req of s) {
    const body = await readRequestBody(req);
    ...
}
const scratchBuffer = new Uint8Array(16 * 1024);
export async function readRequestBody(req: ServerRequest) {
    const contentLength = req.headers.get("Content-length");
    let bufLength = 0;
    if (contentLength) {
        if (!isNaN(+contentLength)) {
            bufLength = +contentLength;
        }
    }

    let ptr = 0;
    let readAmount: number | null = 0;
    let outData: Uint8Array = new Uint8Array(bufLength);

    do {
        readAmount = await req.body.read(scratchBuffer);
        if (readAmount) {
            if (readAmount + ptr > outData.length) {
                const nextOutData = new Uint8Array(readAmount + ptr);
                nextOutData.set(outData.subarray(0, ptr), 0);
                outData = nextOutData;
            }

            outData.set(scratchBuffer.subarray(0, readAmount), ptr);
            ptr += readAmount;
        }
    } while(readAmount);

    return outData;
}

Am I missing some util, or is this the prescribed way? Could we make a util if the latter is correct?

@kitsonk
Copy link
Contributor

kitsonk commented May 22, 2020

await Deno.readAll(req.body)

@ThePrimeagen
Copy link
Author

FeelsGoodMan

@vegerot
Copy link
Contributor

vegerot commented May 22, 2020

This issue should be closed. But I'd just like to point out two things:

  1. This is not documented anywhere (at least so far as Google can find)
  2. This is inconsistent with pretty much every other HTTP framework I know of. In most systems, req.body is pretty consistent behavior.
    Now I know it being inconsistent does not mean it's bad. Every service does things a bit differently, and Deno has opinionated designs (which make it great in the first place), so sometimes you just gotta learn the 🦖 way of doing it. But I think generally it is more intuitive to have req.body be the whole body, and have another member to get it as a reader.

At the very least, instead of not giving you the code, the docs for req.body should mention the Deno.readAll method.

@kitsonk
Copy link
Contributor

kitsonk commented May 23, 2020

It is a fair point that the docs don't make that clear. I believe mostly because Deno.readAll came after the implementation and no one bothered to update the JSDocs, but @cknight has raised #5771 to update that.

std/http isn't a framework, it is still fairly low level implementation of HTTP 1.1 on top of Deno's IO interfaces. Be it HTTP, raw sockets, files, stdin/stdout, etc. they all work of of the Deno.Reader and Deno.Writer interfaces. If you want a framework, there are several to choose from. oak for example has the sort of behaviour you might expect from other HTTP server frameworks for body: https://github.com/oakserver/oak#request

@vegerot
Copy link
Contributor

vegerot commented May 23, 2020

That makes sense. Thank you 🙂

@hansoksendahl
Copy link

@vegerot 🦕 is a better emoji for Deno.

@brandonros
Copy link

deno run --allow-net j2534-http-vehicle.ts 
Check file:///Users/brandonros/Desktop/j2534-http/vehicle-deno/j2534-http-vehicle.ts
error: TS2345 [ERROR]: Argument of type 'ReadableStream<Uint8Array>' is not assignable to parameter of type 'Reader'.
  Property 'read' is missing in type 'ReadableStream<Uint8Array>' but required in type 'Reader'.
    const requestBody = await Deno.readAll(req.body)
                                           ~~~~~~~~
    at file:///Users/brandonros/Desktop/j2534-http/vehicle-deno/j2534-http-vehicle.ts:6:44

    'read' is declared here.
        read(p: Uint8Array): Promise<number | null>;
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        at asset:///lib.deno.ns.d.ts:603:5

import { serve } from "https://deno.land/std@0.126.0/http/server.ts";
import { readerFromStreamReader } from "https://deno.land/std/io/streams.ts"

const handler = async (req: Request): Promise<Response> => {
  if (req.body) {
    const requestBody = await Deno.readAll(req.body)
    console.log(new Uint8Array(requestBody))
  }
  const headers=new Headers({'content-type':'application/octet-stream'});
  const response=new Response(new Uint8Array([65,66,67]), {headers});
  return response;

};

const port = 3000;
console.log(`HTTP webserver running. Access it at: http://localhost:${port}/`);
await serve(handler, { port });

@crowlKats
Copy link
Member

@brandonros req.body isnt a Deno.Reader, but a ReadableStream. if you want to get the whole body at once as a Uint8Array, use req.arrayBuffer()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants