excon: v0.94.0 regression: bignum too big to convert into `long' (RangeError)

Sorry to hear about your troubles here, we had hoped it to be relatively safe, but have started hearing about some issues.

@vasi-stripe here is another intermittent issues, let me know if you have ideas and/or how I can help.

I believe this same issue presents itself as a “malformed header”. This was occurring on this line in Excon 0.94:

excon-0.94.0/lib/excon/response.rb:189:in parse_headers’: malformed header (Excon::Error::ResponseParse)`

Here is the code:

    def self.parse_headers(socket, datum)
      last_key = nil
      until (data = socket.readline.chomp).empty?
        if !data.lstrip!.nil?
          raise Excon::Error::ResponseParse, 'malformed header' unless last_key
          # append to last_key's last value
          datum[:response][:headers][last_key] << ' ' << data.rstrip
        else
          key, value = data.split(':', 2)
          raise Excon::Error::ResponseParse, 'malformed header' unless value

I believe the issue was the chunk size was being parsed as a header so the value was empty. I was able to reproduce this pretty consistently on a request that returned a large payload (via the Shopify Orders.json REST API) starting Excon 0.94.

I tested Excon 0.95 briefly and it seems like it is working. I’ll roll this out to production this week to see if I can reproduce it with a larger volume and report back.

Digging a little deeper, I at least have an initial hypothesis to share. From the stack trace it looks like the failure case related to chunked encoding in particular, which should include a chunk size which is then read. I think with the revisions in 0.94.0 we introduced this regression because some of the reads were made to be more aggressive and buffer. I suspect this may result in a case where we inadvertently read something that isn’t intended to be a chunk size, convert it to an int and then try to do a read. I haven’t definitively confirmed that yet though. Also, I don’t think chunked encoding usage is necessarily the most common in the wild, which may explain why this was happening only infrequently. I’ll try to dig deeper soon, but need to step away for child care duties (family unfortunately got covid over Thanksgiving). I think this gives me some ideas on how/where to look more, but not quite sure when I’ll have time/bandwidth to do so. Certainly welcome further ideas and feedback if folks have them. Thanks!

@geemus Thanks for digging into and addressing this 🙇 I’ll try to update and test this this week and will report back with what I find 😄

I released v0.95.0 this morning, which fixed the other regression. I’m hoping it fixes this one as well, but am unable to easily confirm. Do let me know if you continue to have problems (and/or if you don’t and we can close this). Thanks!

@landongrindheim - I think this commit might help, but it’s hard to know without a clear repro case: https://github.com/excon/excon/commit/912045d0b91731520d68118f0a5ffce2a16aa713. Any confirmation or further information you can garner from that would be most appreciated.

@landongrindheim Yeah, something like that is definitely what I was imagining. Thanks for the extra info. I’ll try to circle back soon and hopefully in the mean time the earlier version should work to fill the gap.

I suspect this may result in a case where we inadvertently read something that isn’t intended to be a chunk size, convert it to an int and then try to do a read.

I spent a little more time tracing through some code and came to a the same conclusion 🙂 In our case this error is happening where we’re handling a bunch of Git metadata. I can imagine a case where a SHA or some other blob is being cast to a very large number. 👇 is the chunk of code I see that potentially happening in.

https://github.com/excon/excon/blob/962956f26c8ce6a9e0ac293dfc2db4db5fc79051/lib/excon/response.rb#L131-L142

@landongrindheim K, thanks. Thought it would be good to at least narrow that down.