Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error reading from socket: Could not parse data #589

Closed
Bonias opened this issue Jan 16, 2020 · 4 comments · Fixed by #590
Closed

error reading from socket: Could not parse data #589

Bonias opened this issue Jan 16, 2020 · 4 comments · Fixed by #590
Labels

Comments

@Bonias
Copy link
Contributor

Bonias commented Jan 16, 2020

Hi,

I'm investigating an error that is raised when making http request to one of the remote APIs. I don't want to post that request as there are some secrets used, but I think I was able to create minimal reproducible example:

require 'socket'

Thread.new do
  server = TCPServer.open(3000)

  client_connection = server.accept

  body_1 = "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nContent-Type: application/json\r\nMyHeader"
  body_2 = ": val\r\n\r\n{}"

  client_connection.write(body_1)
  client_connection.write(body_2)
end

require 'http'
HTTP.get('http://localhost:3000/')

Here is the error that is returned:

Traceback (most recent call last):
       11: from /home/bonias/.rvm/rubies/ruby-2.6.5/bin/irb:23:in `<main>'
       10: from /home/bonias/.rvm/rubies/ruby-2.6.5/bin/irb:23:in `load'
        9: from /home/bonias/.rvm/rubies/ruby-2.6.5/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        8: from (irb):16
        7: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/chainable.rb:20:in `get'
        6: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/chainable.rb:75:in `request'
        5: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/client.rb:31:in `request'
        4: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/client.rb:75:in `perform'
        3: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/connection.rb:103:in `read_headers!'
        2: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/connection.rb:217:in `read_more'
        1: from /home/bonias/.rvm/gems/ruby-2.6.5/gems/http-5.0.0.pre/lib/http/response/parser.rb:31:in `add'
HTTP::ConnectionError (error reading from socket: Could not parse data)

Error is not returned when using http version <= 4.1.1

@ixti ixti added the Bug label Jan 16, 2020
@ixti
Copy link
Member

ixti commented Jan 16, 2020

I confirm this can be reproduced on master, 4-x-stable, v4.3.0, and v4.2.0. No problems on v4.1.1. So it has something to do with http-parser switch.

Related issues and commits

@Bonias
Copy link
Contributor Author

Bonias commented Jan 22, 2020

Here is example reduced to http-rb parser:

require 'http'
parser = HTTP::Response::Parser.new

parser.add("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nContent-Type: application/json\r\nMyHeader")
parser.add(": val\r\n\r\n{}")

it raises IOError: Could not parse data


And here is example reduced to http-parser itself:

# script.rb
require 'http-parser'

class MyParser
  attr_reader :headers

  def initialize
    @state  = HttpParser::Parser.new_instance { |i| i.type = :response }
    @parser = HttpParser::Parser.new(self)
  end

  def add(data)
    return self unless @parser.parse(@state, data)

    raise IOError, "Could not parse data"
  end

  alias << add

  def on_header_field(_response, field)
    puts "[DEBUG] on_header_field: #{field.inspect}"
  end

  def on_header_value(_response, value)
    puts "[DEBUG] on_header_value: #{value.inspect}"
  end

  def on_headers_complete(_response)
    puts "[DEBUG] on_headers_complete"
  end

  def on_body(_response, chunk)
    puts "[DEBUG] on_body: #{chunk.inspect}"
  end

  def on_message_complete(_response)
    puts "[DEBUG] on_message_complete"
  end
end

parser = MyParser.new

parser.add("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nContent-Type: application/json\r\nMyHeader")
puts "-------"
parser.add(": val\r\n\r\n{}")

# → ruby script.rb 
# [DEBUG] on_header_field: "Content-Length"
# [DEBUG] on_header_value: "2"
# [DEBUG] on_header_field: "Content-Type"
# [DEBUG] on_header_value: "application/json"
# [DEBUG] on_header_field: "MyHeader"
# -------
# [DEBUG] on_header_field: ""
# [DEBUG] on_header_value: "val"
# [DEBUG] on_headers_complete
# [DEBUG] on_body: "{}"
# [DEBUG] on_message_complete

The problem is obvious now:

# [DEBUG] on_header_field: ""

This means that @headers.add('', 'val') is executed by http-rb when header value is received and it raises an exception:

> HTTP::Headers.new.add('', 'val')
HTTP::HeaderError: Invalid HTTP header field name: ""

and because of this parser fails. http-parser should not return empty header field in the first place, but maybe we should add workaround in http-rb (just check if field is not empty - https://github.com/httprb/http/blob/master/lib/http/response/parser.rb#L52)?

@Bonias
Copy link
Contributor Author

Bonias commented Jan 22, 2020

http-parser should not return empty header field in the first place, but maybe we should add workaround in http-rb (just check if field is not empty - https://github.com/httprb/http/blob/master/lib/http/response/parser.rb#L52)?

I think I was wrong. Apparently this is just how http-parser works. See another example:

parser = MyParser.new
body = "HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"
body.each_char { |char|  parser.add(char) }

# [DEBUG] on_header_field: "C"
# [DEBUG] on_header_field: "o"
# [DEBUG] on_header_field: "n"
# [DEBUG] on_header_field: "t"
# [DEBUG] on_header_field: "e"
# [DEBUG] on_header_field: "n"
# [DEBUG] on_header_field: "t"
# [DEBUG] on_header_field: "-"
# [DEBUG] on_header_field: "L"
# [DEBUG] on_header_field: "e"
# [DEBUG] on_header_field: "n"
# [DEBUG] on_header_field: "g"
# [DEBUG] on_header_field: "t"
# [DEBUG] on_header_field: "h"
# [DEBUG] on_header_field: ""
# [DEBUG] on_header_value: "2"
# [DEBUG] on_header_value: ""
# [DEBUG] on_headers_complete
# [DEBUG] on_body: "{"
# [DEBUG] on_body: "}"
# [DEBUG] on_message_complete

So http-rb desn't use http-parser correctly. It should build header keys and values from the chunks, exactly like body is built at the moment. I'll try to fix this.

@ixti
Copy link
Member

ixti commented Jan 22, 2020

Thank you once again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants