Giraffe: jsonChunked does not follow RFC

Created on 31 Jul 2019  路  1Comment  路  Source: giraffe-fsharp/Giraffe

The jsonChunked function does not follow the RFC for Transfer-Encoding: chunked.

jsonChunked calls SerializeToStreamAsync on the current JsonSerializer.

https://github.com/giraffe-fsharp/Giraffe/blob/6e33155d3f0929af52886a7302fbd12a07ff9cb7/src/Giraffe/ResponseWriters.fs#L156-L163

Which SerializeToStreamAsync's implementation is writing directly to the output stream

https://github.com/giraffe-fsharp/Giraffe/blob/7981279dfef1432b271975449c8fcb5935cf977c/src/Giraffe/Serialization.fs#L95-L99


If you use curl to hit an endpoint using jsonChunked you'll get an error

* Illegal or missing hexadecimal sequence in chunked-encoding
* stopped the pause stream!
* Closing connection 0
curl: (56) Illegal or missing hexadecimal sequence in chunked-encoding

Similarly browsers and javascript libraries don't know how handle this response. The problem is it doesn't follow the RFC for Transfer-Encoding: chunked.

The RFC BNF is shown below

       Chunked-Body   = *chunk
                        last-chunk
                        trailer
                        CRLF

       chunk          = chunk-size [ chunk-extension ] CRLF
                        chunk-data CRLF
       chunk-size     = 1*HEX
       last-chunk     = 1*("0") [ chunk-extension ] CRLF

       chunk-extension= *( ";" chunk-ext-name [ "=" chunk-ext-val ] )
       chunk-ext-name = token
       chunk-ext-val  = token | quoted-string
       chunk-data     = chunk-size(OCTET)
       trailer        = *(entity-header CRLF)

An example of this is from MDN

HTTP/1.1 200 OK 
Content-Type: text/plain 
Transfer-Encoding: chunked

7\r\n
Mozilla\r\n 
9\r\n
Developer\r\n
7\r\n
Network\r\n
0\r\n 
\r\n

If we're going to keep the interface of writing to a Stream for the json serializer, we're going to have to implement some type of Stream that handles writing this format to the HttpContext's response stream.

PR approved bug help wanted

Most helpful comment

I'm going to close this on account of #369 being merged, if anything still needs to happen best to open a new issue!

>All comments

I'm going to close this on account of #369 being merged, if anything still needs to happen best to open a new issue!

Was this page helpful?
0 / 5 - 0 ratings