crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.22k stars 1.61k forks source link

FormData part.body.read does not fill buffer #14601

Closed mohd-akram closed 1 month ago

mohd-akram commented 1 month ago

Bug Report

I had this bit of working code before, but the behavior seems to have changed at some point. Run this and open Chrome, paste a good amount of text and submit the form. You should get a 400 error. For some reason part.body.read does not attempt to fill the buffer even though there's plenty of space left. I expect it to return size < slice size only if there isn't more data to read. This was the behavior before.

require "http/server"

server = HTTP::Server.new do |context|
  path = context.request.path

  case context.request.method
  when "GET"
    context.response.content_type = "text/html; charset=utf-8"
    context.response.print <<-HTML
    <form method=POST enctype=multipart/form-data>
    <textarea name=data rows=50 cols=100></textarea><br><br>
    <input type=submit>
    </form>
    HTML
  when "POST"
    case path
    when "/"
      data = nil
      HTTP::FormData.parse(context.request) do |part|
        case part.name
        when "data"
          body = Bytes.new 1024
          size = part.body.read body
          puts "read", size
          if part.body.read_byte.nil?
            puts "read as much as we could!"
          elsif size < 1024 && part.body.read body
            puts "could still read more!"
          end
        end
      end
      unless data
        context.response.status_code = 400
        context.response.print "Bad Request"
      end
    end
  end
end

port = 8080

server.bind_tcp port
puts "Listening on http://127.0.0.1:#{port}"
server.listen
straight-shoota commented 1 month ago

Per documentation, IO#read does not guarantee to fill the entire slice. It reads as much data as is immediately available on the socket buffer. This can of course differ depending on utilization of the connection which explains why the code may fill the entire slice sometimes but sometimes not.

If you want to ensure filling the slice, you'll need to do repeated reads until there is no data left. IO#read_fully can be useful for that, but it expects there to be enough data available for a fill. You might also consider IO.copy which has no size requirements.

mohd-akram commented 1 month ago

Thanks for clarifying. IO.copy seems to have the same caveat ("at most"). It would be nice to have a method that reads as much as possible up to a limit. It seems #11896 might address this.

straight-shoota commented 1 month ago

IO.copy continues reading until there is no more data available (i.e. IO#read returns 0).

It seems the documentation needs some clarification.