Kemal gets stuck for long response bodies

[marzhaev]

Description

After upgrading to Crystal 0.35.0 on Kemal's master branch I'm having issues with responding with long strings.

Steps to Reproduce

1. Follow official install instructions.
2. Create a route such as get "/" { "my_string," * 1_000 }
3. Launch Kemal app.
4. I use curl to curl localhost:3000

Expected behavior: I expect the app to return a long self-repeating string with a connection properly closed.

Actual behavior: My curl get stuck forever printing only the first chunk. Obviously, it works as buggy in browser.

Reproduces how often: Seems to happen for a string that needs to be chunked as per Transfer-Encoding requirements.

Versions

Crystal 0.35.0 (2020-06-09)

LLVM: 10.0.0
Default target: x86_64-apple-macosx

Persists for both OSs: macOS Catalina 10.15.4 and Ubuntu 18.04

[marzhaev]

Crystal core interacted with HTTP::Server::Response by introducing @chunked. Kemal extends HTTP::Server::Response's Output with its own implementation of close that plays with content_length.

Can we remove this code? I'm unclear on its use-cases.

[kostya]

same bug

[grvm]

Same bug

Crystal 0.35.1
Manjaro 20.0.3
Kemal (0.26.1 at a8c0f09/master). Warning: Zlib is deprecated, use Compress::Zlib with 0.26.1

Any response above 8kb is chunked & the connection is stuck. Tried specifying the content_length as well with no luck.

[s0kil]

Mints core testing is also broken due to this bug, the browser awaits a WS response from the server that never arrives, timed out.

[bcardiff]

FYI, until #576 is merged, in order to apply that patch locally you can include the following in your project somewhere after require "kemal"

class HTTP::Server::Response
  class Output
    # original definition since Crystal 0.35.0
    def close
      return if closed?

      unless response.wrote_headers?
        response.content_length = @out_count
      end

      ensure_headers_written

      super

      if @chunked
        @io << "0\r\n\r\n"
        @io.flush
      end
    end

    # patch from https://github.com/kemalcr/kemal/pull/576
    def close
      # ameba:disable Style/NegatedConditionsInUnless
      unless response.wrote_headers? && !response.headers.has_key?("Content-Range")
        response.content_length = @out_count
      end

      ensure_headers_written

      previous_def
    end
  end
end

Eventually if/when crystal std-lib http improves its support for Content-Range the monkey-patch in kemal itself will be able to go away.

[carcinocron]

After ubuntu updated my crystal to 0.35 unexpectedly (lol), My kemal app suddenly started having this behavior where chrome would show the "page loading" spinner indefinitely. And it was difficult to see responses in the devtools. They would visibly show up in the normal browser, but no data would show up in chrometools. It seems as kemal did not close the request correctly, causing chrome to anticipate more data even though it was complete.

This new behavior was fixed by this snippet (posted above) #575 (comment)