Ensure Content-Length is set for all one-shot responses

Sone clients won't correctly operate if a `Content-Length: 0` header isn't given when returning replies for things like redirect. `curl` is an example of this.

.gitignore

@@ -5,3 +5,4 @@ tags
 /.coverage/
 /_corral/
 /_repos/
+/lock.json

.release-notes/next-release.md

@@ -0,0 +1,56 @@
+## Ensure Content-Length is set for all Responses that need it
+
+Previously responses without explicitly added Content-Length didn't add that header with a `0` value. This made some HTTP clients hang.
+Now all responses built with this library will have a default `Content-Length` header set, unless marked with `Transfer-Encoding: chunked`
+
+## Added `ResponseBuilderHeaders.set_content_length(content_length: USize)`
+
+This way it is more convenient to set a content-length from a numeric value. E.g. from the size of a prepared array to be passed as HTTP body:
+
+```pony
+let body = "I am a teapot"
+let response = 
+  Responses.builder()
+    .set_status(StatusOK)
+    .set_content_length(body.size())
+    .add_header("Content-Type", "text/plain")
+    .finish_headers()
+    .add_chunk(body)
+    .build()
+```
+
+## Added `BuildableResponse.delete_header(header_name: String)`
+
+Previously it was not possible to delete a header, once set it was permanent. No it is possible to delete a header e.g. in multi-stage processing of a HTTP response.
+
+## `ResponseBuilderBody.add_chunk()` now takes a `ByteSeq` instead of `Array[U8] val`
+
+This allows to pass `String val` as well as `Array[U8] val` to `add_chunk`.
+
+```pony
+let response = Responses.builder()
+  .set_content_length(7)
+  .finish_headers()
+  .add_chunk("AWESOME")
+  .build()
+```
+
+## `BuildableResponse.create()` now only takes a `Status` and optionally a `Version`
+
+The logic applied to set `content_length` and `transfer_encoding` from the constructor parameters was a bit brittle, so it got removed. Use both `set_content_length(content_length: USize)` and `set_transfer_encoding(chunked: (Chunked | None))` to set them:
+
+```pony
+let body = "AWESOME"
+let response = BuildableResponse
+  .create(StatusOK)
+  .set_content_length(body.size())
+  .set_header("Content-Type", "text/plain")
+```
+
+## `Response.transfer_coding()` changed to `.transfer_encoding()`
+
+The wording now is now equal to the actual header name set with this method.
+
+## `BuildableResponse.set_transfer_coding()` changed to `.set_transfer_encoding()`
+
+Following the `Response` trait.

CHANGELOG.md

@@ -6,12 +6,19 @@ All notable changes to this project will be documented in this file. This projec
 
 ### Fixed
 
+- Ensure Content-Length is set for all Responses that need it ([PR #74](https://github.com/ponylang/http_server/pull/74))
 
 ### Added
 
+- Added `ResponseBuilderHeaders.set_content_length(content_length: USize)` ([PR #74](https://github.com/ponylang/http_server/pull/74))
+- Added `BuildableResponse.delete_header(header_name: String)` ([PR #74](https://github.com/ponylang/http_server/pull/74))
 
 ### Changed
 
+- `ResponseBuilderBody.add_chunk()` now takes a `ByteSeq` instead of `Array[U8] val` ([PR #74](https://github.com/ponylang/http_server/pull/74))
+- `BuildableResponse.create()` now only takes a `Status` and a `Version` ([PR #74](https://github.com/ponylang/http_server/pull/74))
+- `BuildableResponse.set_transfer_coding()` changed to `.set_transfer_encoding()` ([PR #74](https://github.com/ponylang/http_server/pull/74))
+- - `Response.transfer_coding()` changed to `.transfer_encoding()` ([PR #74](https://github.com/ponylang/http_server/pull/74))
 
 ## [0.4.6] - 2024-01-14
 

Makefile

@@ -3,7 +3,8 @@ config ?= release
 PACKAGE := http_server
 GET_DEPENDENCIES_WITH := corral fetch
 CLEAN_DEPENDENCIES_WITH := corral clean
-COMPILE_WITH := corral run -- ponyc
+PONYC ?= ponyc
+COMPILE_WITH := corral run -- $(PONYC)
 
 BUILD_DIR ?= build/$(config)
 SRC_DIR ?= $(PACKAGE)

http_server/_ignore_ascii_case.pony

@@ -0,0 +1,79 @@
+primitive IgnoreAsciiCase
+  """
+  Compares two strings lexicographically and case-insensitively.
+  Only works for ASCII strings.
+  """
+
+  fun compare(left: String, right: String): Compare =>
+    """
+
+    Less: left sorts lexicographically smaller than right
+    Equal: same size, same content
+    Greater: left sorts lexicographically higher than right
+
+    _compare("A", "B") ==> Less
+    _compare("AA", "A") ==> Greater
+    _compare("A", "AA") ==> Less
+    _compare("", "") ==> Equal
+    """
+    let ls = left.size()
+    let rs = right.size()
+    let min = ls.min(rs)
+
+    var i = USize(0)
+    while i < min do
+      try
+        let lc = _lower(left(i)?)
+        let rc = _lower(right(i)?)
+        if lc < rc then
+          return Less
+        elseif rc < lc then
+          return Greater
+        end
+      else
+        Less // should not happen, size checked
+      end
+      i = i + 1
+    end
+    // all characters equal up to min size
+    if ls > min then
+      // left side is longer, so considered greater
+      Greater
+    elseif rs > min then
+      // right side is longer, so considered greater
+      Less
+    else
+      // both sides equal size and content
+      Equal
+    end
+
+  fun eq(left: String, right: String): Bool =>
+    """
+    Returns true if both strings have the same size
+    and compare equal ignoring ASCII casing.
+    """
+    if left.size() != right.size() then
+      false
+    else
+      var i: USize = 0
+      while i < left.size() do
+        try
+          if _lower(left(i)?) != _lower(right(i)?) then
+            return false
+          end
+        else
+          return false
+        end
+        i = i + 1
+      end
+      true
+    end
+
+  fun _lower(c: U8): U8 =>
+    if (c >= 0x41) and (c <= 0x5A) then
+      c + 0x20
+    else
+      c
+    end
+
+

http_server/_pending_responses.pony

@@ -29,7 +29,7 @@ type _PendingResponse is (RequestID, Vec[_ByteSeqs])
 
 class ref _PendingResponses
   // TODO: find out what is the most efficient way to
-  //       keep and acucmulate a pending response
+  //       keep and accumulate a pending response
   //       from ByteSeq and ByteSeqIter
   embed _pending: Array[_PendingResponse] ref = _pending.create(0)
 

http_server/_test_headers.pony

@@ -1,3 +1,4 @@
+use "collections"
 use "debug"
 use "pony_check"
 use "pony_test"
@@ -9,6 +10,8 @@ actor \nodoc\ _HeaderTests is TestList
 
   fun tag tests(test: PonyTest) =>
     test(Property1UnitTest[Array[Header]](_HeadersGetProperty))
+    test(Property1UnitTest[Set[String]](_HeadersDeleteProperty))
+
 
 class \nodoc\ iso _HeadersGetProperty is Property1[Array[Header]]
   fun name(): String => "headers/get/property"
@@ -46,3 +49,46 @@ class \nodoc\ iso _HeadersGetProperty is Property1[Array[Header]]
         end
       end
     end
+
+
+class \nodoc\ iso _HeadersDeleteProperty is Property1[Set[String]]
+  fun name(): String => "headers/delete/property"
+
+  fun gen(): Generator[Set[String]] =>
+    // we need unique values in our set, lower and upper case letters are
+    // considered equal for our Headers impl, so we need to avoid e.g. `a` and
+    // `A` as the set thinks they are different, but Headers not.
+    Generators.set_of[String](Generators.ascii(where max=10, range=ASCIILettersLower))
+
+  fun property(sample: Set[String], h: PropertyHelper) =>
+    let headers = Headers.create()
+
+    let added: Array[String] = Array[String](sample.size())
+    let iter = sample.values()
+    try
+      let first = iter.next()?
+      for header in iter do 
+          headers.add(header, header)
+          added.push(header)
+      end
+
+      h.log("Added headers:" where verbose = true)
+      for a in added.values() do
+        h.log(a where verbose = true)
+      end
+
+      // the header we never added is not inside
+      h.assert_true(headers.delete(first) is None, "Header: " + first + " got deleted from headers although never added")
+      for added_header in added.values() do
+
+        // available before delete
+        h.assert_true(headers.get(added_header) isnt None, "Header: " + added_header + " was added to headers, but wasn't retrieved with get")
+        h.assert_true(headers.delete(added_header) isnt None, "Header: " + added_header + " was added to headers, but wasn't found during delete")
+        // gone after delete
+        h.assert_true(headers.get(added_header) is None, "Header: " + added_header + " was deleted but could be retrieved with get")
+
+        // the header we never added is not inside
+        h.assert_true(headers.delete(first) is None, "Header: " + first + " got deleted from headers although never added")
+      end
+    end
+

http_server/_test_response.pony

@@ -0,0 +1,87 @@
+use "pony_test"
+
+actor \nodoc\ _ResponseTests is TestList
+  new make() =>
+    None
+
+  fun tag tests(test: PonyTest) =>
+    test(_BuildableResponseTest)
+    test(_ResponseBuilderTest)
+
+class \nodoc\ iso _BuildableResponseTest is UnitTest
+  fun name(): String => "responses/BuildableResponse"
+
+  fun apply(h: TestHelper) =>
+    let without_length = BuildableResponse()
+    h.assert_true(without_length.header("Content-Length") is None, "Content-Length header set although not provided in constructor")
+
+    let array = recover val String.from_iso_array(without_length.array()) end
+    h.log(array)
+    h.assert_true(array.contains("\r\nContent-Length: 0\r\n"), "No content-length header added in array response")
+
+    let bytes = without_length.to_bytes().string()
+    h.log(bytes)
+    h.assert_true(bytes.contains("\r\nContent-Length: 0\r\n"), "No content-length header added in to_bytes response")
+
+
+    let with_length = BuildableResponse().set_content_length(42)
+    match with_length.header("Content-Length")
+    | let hvalue: String =>
+      h.assert_eq[String]("42", hvalue)
+    | None =>
+      h.fail("No Content-Length header set")
+    end
+
+    let chunked_without_length = BuildableResponse().set_transfer_encoding(Chunked)
+    h.assert_true(without_length.header("Content-Length") is None, "Content-Length header set although not provided in constructor")
+
+    let array2 = recover val String.from_iso_array(chunked_without_length.array()) end
+    h.log(array2)
+    h.assert_false(array2.contains("\r\nContent-Length: "), "Content-length header added in array response although chunked")
+
+    let bytes2 = chunked_without_length.to_bytes().string()
+    h.log(bytes2)
+    h.assert_false(bytes2.contains("\r\nContent-Length: "), "Content-length header added in to_bytes response although chunked")
+
+    // first set content-length, then transfer coding
+    let complex =
+      BuildableResponse().set_content_length(42).set_transfer_encoding(Chunked)
+    h.assert_true(complex.header("Content-Length") is None, "Content-Length header set although not provided in constructor")
+
+    let array3 = recover val String.from_iso_array(complex.array()) end
+    h.log(array3)
+    h.assert_false(array3.contains("\r\nContent-Length: "), "Content-length header added in array response although chunked")
+
+    let bytes3 = complex.to_bytes().string()
+    h.log(bytes3)
+    h.assert_false(bytes3.contains("\r\nContent-Length: "), "Content-length header added in to_bytes response although chunked")
+
+
+class \nodoc\ iso _ResponseBuilderTest is UnitTest
+  fun name(): String => "responses/ResponseBuilder"
+
+  fun apply(h: TestHelper) =>
+    let without_length = Responses.builder().set_status(StatusOK).add_header("Server", "FooBar").finish_headers().build()
+    var s = String.create()
+    for bs in without_length.values() do
+      s.append(bs)
+    end
+    h.assert_true(s.contains("\r\nContent-Length: 0\r\n"), "No content length added to Request: " + s)
+
+    let with_length = Responses.builder().set_status(StatusOK).set_content_length(4).finish_headers().add_chunk("COOL").build()
+    s = String.create()
+    for bs in with_length.values() do
+      s.append(bs)
+    end
+    h.assert_true(s.contains("\r\nContent-Length: 4\r\n"), "No or wrong content length added to Request: " + s)
+
+    let chunked =
+      Responses.builder().set_status(StatusOK).set_transfer_encoding(Chunked).add_header("Foo", "Bar").finish_headers().add_chunk("FOO").add_chunk("BAR").add_chunk("").build()
+    let c = recover val
+      let tmp = String.create()
+      for bs in chunked.values() do
+        tmp.append(bs)
+      end
+      tmp
+    end
+    h.assert_eq[String]("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nFoo: Bar\r\n\r\n3\r\nFOO\r\n3\r\nBAR\r\n0\r\n\r\n", c)