Godot: HTTPClient is_response_chunked (seemingly) always returns false

[nathanfranke]

Godot version:
3.2 Release

OS/device including version:
Linux Mint 19.3

Issue description:
httpclient.is_response_chunked() == false even though the response is split into chunks

Steps to reproduce:
Load a file >4096 bytes and read the output. It will be chunked regardless of is_response_chunked()

const MY_HOST := "https://gist.githubusercontent.com"
const MY_EP := "/The5heepDev/a15539b297a7862af4f12ce07fee6bb7/raw/7164813a9b8d0a3b2dcffd5b80005f1967887475/entire_bee_movie_script"

func _ready() -> void:
    var httpclient := HTTPClient.new()

    assert(httpclient.connect_to_host(MY_HOST, -1, true) == OK)
    while httpclient.get_status() == HTTPClient.STATUS_RESOLVING or httpclient.get_status() == HTTPClient.STATUS_CONNECTING:
        assert(httpclient.poll() == OK)
        yield(Engine.get_main_loop(), "idle_frame")

    print(MY_HOST + MY_EP)

    assert(httpclient.request(HTTPClient.METHOD_GET, MY_EP, []) == OK)
    while httpclient.get_status() == HTTPClient.STATUS_REQUESTING:
        assert(httpclient.poll() == OK)
        yield(Engine.get_main_loop(), "idle_frame")

    print()
    print("Chunked?: " + str(httpclient.is_response_chunked()))
    print("Full Body: " + str(httpclient.get_response_body_length()))
    print()
    while httpclient.get_status() == HTTPClient.STATUS_BODY:
        var size := httpclient.read_response_body_chunk().size()
        if size != 0:
            print("Chunk: " + str(size))

Minimal reproduction project:
Test7.zip

[follower]

Some additional context for this issue:

Overlapping use of "chunk" terminology

As I discovered when investigating a different issue it seems that the HTTPClient code seems to use the term "chunk" to refer to two different concepts:

1) "chunk" in the sense of _HTTP's "Chunked transfer encoding"_; and,

2) "chunk" in the generic sense of _"some quantity of bytes"_.

The documentation doesn't currently clarify which meaning applies in each case and the implementation also obscures the difference.

Matching properties/methods to each meaning of "chunk"

So, as I understand it:

• is_response_chunked() docs would be more accurate written as[0]:

  If true, this HTTPClient has a response that has a HTTP Transfer-Encoding of chunked.

  This is based on these sections of code: https://github.com/godotengine/godot/blob/c358399f81bedd0c4a4a765b56a7c40a57c4f5f3/core/io/http_client.cpp#L457-L461 https://github.com/godotengine/godot/blob/c358399f81bedd0c4a4a765b56a7c40a57c4f5f3/core/io/http_client.cpp#L242-L245

• read_chunk_size does not refer to HTTP chunked encoding but rather the "some quantity of bytes" sense (as per the docs for read_chunk_size: "The size of the buffer used and maximum bytes to read per iteration").

• read_response_body_chunk ( ) somewhat confusingly relates to both senses of the term chunk--the specific meaning is dependent on whether the response uses HTTP Chunked Transfer Encoding or not!

  Implementation of read_response_body_chunk ( ) when Chunked Transfer Encoding is used: https://github.com/godotengine/godot/blob/c358399f81bedd0c4a4a765b56a7c40a57c4f5f3/core/io/http_client.cpp#L527-L634

  Implementation of read_response_body_chunk ( ) when Chunked Transfer Encoding is not used: https://github.com/godotengine/godot/blob/c358399f81bedd0c4a4a765b56a7c40a57c4f5f3/core/io/http_client.cpp#L634-L658

• Note also that AFAICT when Chunked Transfer Encoding is used, the value of read_chunk_size is ignored and the number of bytes returned is determined by the length of the size of the Chunked Transfer Encoding response.

  This observation is based on this section of code: https://github.com/godotengine/godot/blob/c358399f81bedd0c4a4a765b56a7c40a57c4f5f3/core/io/http_client.cpp#L554-L628

  Ideally this would be clarified/documented in the read_chunk_size & read_response_body_chunk() docs at a minimum.

Summary

Thus, based on my understanding this isn't a bug but is a documentation issue.

(In theory it might be considered an implementation wart that the read_chunk_size value is ignored when Chunked Transfer Encoding is used.)

Hope this is some helpful context.

---

[0] Although the wording could definitely still be improved to be both accurate and actually helpful. :D

[jonbonazza]

All of follower's analysis is correct. I do want to clarify that as per the http spec, when chunked transfer encoding is used, the chunk size should always be derived from the beginning of the response body. This is why "read_chunk_suze is ignored in this case. I agree that variable's name could be less conflated though.