Tiny_httpd.Response.make_raw Unconditionally adds Content-Length header even if provided by caller.

[zoj613]

There is a subtle bug in the above function. It disregards any content-length info passed to the make_raw function and adds its own content-length header that "hides" the one given by the caller. This is a problem if I write the call like so:

  Tiny_httpd.Response.make_raw ~headers:[("content-length", "10")] ~code:200 ""

Since I give an empty body and encode the size of the resource in the defined header, I expect the response to contain "10" for the content-length header, however in the current implementation is will be "0". This is because make_raw is implemented as:

tiny_httpd/src/core/response.ml

Lines 15 to 20 in b80c5f9

	let make_raw ?(headers = []) ~code body : t =
	(* add content length to response *)
	let headers =
	Headers.set "Content-Length" (string_of_int (String.length body)) headers
	in
	{ code; headers; body = `String body }

. It unconditionally adds another duplicated content-length header with the wrong value. This is bad because the implementation of Header.set prepends the new wrong header value at the head of the list like so:

tiny_httpd/src/core/headers.ml

Lines 22 to 24 in b80c5f9

	let set x y h =
	let x' = String.lowercase_ascii x in
	(x', y) :: List.filter (fun (k, _) -> k <> x') h

An unsuspecting user like myself will run the following code on the response header expecting to get the correct content-length:

  match List.assoc_opt "content-length" headers with
  | Some l -> int_of_string l
  | None -> ...

In the above snippet I expect l to be "10" but its "0".

I think it's best to first check if ?headers parameter contains a content-length value before adding one inside the make_raw function to avoid this subtle bug. This is especially important when writing a HEAD method handler like so:

        S.add_route_handler server ~meth:`HEAD S.Route.rest_of_path_urlencoded (fun path _ ->
          match V.file_size path with
          | None -> S.Response.make_raw ~code:404 "Not found"
		  | Some l -> S.Response.make_raw ~headers:[("content-length", string_of_int l)] ~code:200 ""

where I don't have access to the resource contents and only have its size via VFS.file_size. To workaround this i'm forced to basically return a dummy body of length l just to make this implementation of make_raw return a sensible result.

Let me know what you think is the best way to move forward.

[c-cube]

I don't think it's a bug, because this function also defines the body to "". The content length *must* match the body's actual length. If your use case requires more control over the body, I'd suggest one of the streaming body functions? You also wouldn't set the content length but it'd use chunked encoding to allow you to write the body as you produce it.