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

Can't convert Array into String. (TypeError) #585

Closed
dentarg opened this issue Jan 13, 2020 · 7 comments · Fixed by #587
Closed

Can't convert Array into String. (TypeError) #585

dentarg opened this issue Jan 13, 2020 · 7 comments · Fixed by #587
Labels

Comments

@dentarg
Copy link
Contributor

dentarg commented Jan 13, 2020

This looks like a bug in the HTTP parser used (https://github.com/tmm1/http_parser.rb), but I thought I would first report it here. (I haven't yet taken the time to reproduce it with only http_parser.rb.)

The bug happens when you try to follow really long redirect URLs. In my case the URL have the whole page content embedded, that's why it's so long (see https://github.com/alcor/itty-bitty/). https://burd.se/httprb2 is an example that redirect to such an URL.

Repro:

$ git clone git@github.com:httprb/http.git
$ cd http
$ docker run --rm -it -v $(pwd):/app ruby:2.6 bash
root@8b3b9a934e9b:/# cd /app
root@8b3b9a934e9b:/app# gem install bundler
root@8b3b9a934e9b:/app# bundle
root@8b3b9a934e9b:/app# gem build http
root@8b3b9a934e9b:/app# gem install http-5.0.0.pre.gem
root@8b3b9a934e9b:/app# ruby -rhttp -e 'puts HTTP::VERSION; HTTP.follow.get("https://burd.se/httprb2")'
5.0.0.pre
Traceback (most recent call last):
	7: from -e:1:in `<main>'
	6: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/chainable.rb:20:in `get'
	5: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/client.rb:34:in `request'
	4: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/redirector.rb:61:in `perform'
	3: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/redirector.rb:98:in `redirect_to'
	2: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/request.rb:106:in `redirect'
	1: from /usr/local/lib/ruby/2.6.0/forwardable.rb:230:in `join'
/usr/local/bundle/gems/addressable-2.7.0/lib/addressable/uri.rb:1884:in `join': Can't convert Array into String. (TypeError)
I suspect it is the parser, because with this patch
diff --git a/lib/http/response/parser.rb b/lib/http/response/parser.rb
index 19989d5..caa8412 100644
--- a/lib/http/response/parser.rb
+++ b/lib/http/response/parser.rb
@@ -53,6 +53,7 @@ module HTTP
       end

       def on_header_value(_response, value)
+        p value
         @headers.add(@field, value) if @field
       end
This is the output
root@8b3b9a934e9b:/app# ruby -rhttp -e 'puts HTTP::VERSION; HTTP.follow.get("https://burd.se/httprb2")'
5.0.0.pre
"Mon, 13 Jan 2020 16:38:18 GMT"
"text/html;charset=utf-8"
"chunked"
"close"
"__cfduid=d0408cdfd6fe21b1ab4b6adde29adb2921578933498; expires=Wed, 12-Feb-20 16:38:18 GMT; path=/; domain=.burd.se; HttpOnly; SameSite=Lax"
"https://itty.bitty.site/#/?XQAAAAKhFgAAAAAAAAAeGQknlxhIQakTrqORUwLuwNEJhII5BpfZZe5tdT5QIwTSvOsK2m3Bfw5s9+y2qRxSY1BcMjsN6DS6cL01kzF+/htSy2Bl4kCeWPe2MR3DiTiQ+y3SwAXN7Cxy+UVIJS6RNKq2PuLesCU9Q+7WTJVZ0Av/eNbp5ES4W/mXBdTuS/ePyhh5VlUKyMKkTqakZqYXgeS9YtMYinYS5oQhdkYruc4Bv2D3r621D2i5JheDlF7kTQ3uZSTtoWW+y8/ox2KYVZP8j5YOLwJTNQSLYdZ8im1nJynRN0UhzX0ztHlae7rDee5gTzByVPXv+c6Xd7UHSsdhMdgY2WSG67yQ1Zs1jPGX3daqF5VGETN1aUwCJbKa1ke7yxhlMdatQ9p8t3V4ISbW4DPyStoGlk87MfFyqpZOqL/nzUWOeHqHeb77VP/GQn83m9phRjBBkImblZSdIKAMJE4+K/Df4YWH0zlor7WAnO1dwS8hXYkY5FiCSCPKZ888EQgOeQhf3tBPe9NK7HX54cl2BN4AzBFjSRPnGFEa3cbD6cm2zmmpDoMfF+BD0H9Xv9JTegg8kG8T+erwf8I7/lvs9AKpFa2dyT/Pn2RNM2ZUHgLnhmbAKpms/lWmk5HZ355T3HSue4WSvnd8VFGxvRg1fnzh4+uqT3GeYHpak0XRE4FXtdvnNml3WUVER/GeAfeiXOw2UGXDuneHfhaLPam9xuVaL1YX54fwVQ/iWBjPzc9ZBFO5AKm2BbqCmBykyBYQL9mrO6AHG2TqxbAULXb3YOFfITtHTokI7lmr7Vi6iA6uj+3LWdcHEieXgdR42tAOetFjcxnkg2O8ebYgs0kO+aklrZP8/V6Q1UE858rNDz9ejavwc20k+bh5UwL7Ls4VSOk3Yqvrk3wG1aMsOA6V9vxk6nyPLtR8oBXkbhE5ElHiCYd1qd6f94JiRUWcqgfCSNDYes8pWifn9tj9YE2ZygFJ2e2l8mmMcnSQ0m1oXEGYnzY9dnBGEmL9cHYzppASVxner"
"AYHjVVPcUX8l6Zh8txIrDwWJU8RKjBwYRYq0yYW9X3bP5gumSWRPT7rConR1ulDEHhrHiF2WbF366uigOO/dHffQYtxCOA0ep5wy4nhuHYnPLuCsc/aeaAktAWZXOiQ8W/obYr9Ng52dfmILf6X8o56O8h5vvhD21NOzRsTVCiA/l7qMutRhB87GYP5kQ/DlMy27u8xeQ8pip4rPwinxaWDx5BkSIMTcFHHa5B0Xn9PUlUBXeugCuE2Onqf34SMgp3vxMEsIZBeU942SBysVYrmvPFS0T1pwZU8qiwIX4H8T7kxGzyp9CcomeFnWJb24slRzSglUV9LkyAQ4NibfOBwFQCcp58YSTMtYVytkQGV2a3OxivDXxPdewmrM7mx4X9Ezvz5xO2u0a/h2nMBRGk1xeOwLH14CTOTN1MBbYr+zQysRBxB+xtHIIxhBz/UdoZeVnIwGltv79NbukWn1+J+56Aafch3PQ6HVuoYG2t1ZUYOxLOr42fRwMKuEWIZ+D1zRIcn0jUVBv7I2NT3F6FmcHMGE/Mop3zGKSQ/J4R9Lg1DUgHAdzs6ikdgNXderLCnK/r9VBuR+k35hp0jPcs9B6mWPH7Fg4uoxZDgT438nDMj1F/gn222+vMKjf6x1vQlcB9OqzCabhVzAhAV3Q187RO/fNtVBJ+K/j2I+Bsa1dWFuWsmknieGZ7vdaJne0BcO1KJ+A3VkvC2DAsJlcR9PABdBgQesNzMONmXTkLkPwP0FWrQ1g2Wk6poeBX3NVKkN0/uNwq4hsXOf5ITiO5ZnOeGXF+LPQ9OKh8jXeaqBnRYXAHI2qHmqavNv7qLtygi8tFBIMjPHG3y4bfX3IhGhQFdreYLx/Rclse1Ohkp/FzI+5YVsTc7t6aegSHvRUsZX7KcroTvOT/Bbr0b/JUbuacrh3esQ+OsxWNGtImSe/YJHl7Hgdjz3VONC2QauizMswXT77ITbwl5xc7RBJ0eDKgKhoBdLrJfyOuhDxuWV7xn4R2Z86PgfQ3OfE6ngbaH4MswD/+NK3e6Ss48LHWI/tmrk6z16O4S5UztIt6IeaQvwn2jxN1qbI7KrZk9QZ1ZseLv4ELjbbU0inAJqzIYXxV3rQoMFWasBKTI/csjhnpMCmbQHTjeJvl+TECNqsj4Ay/D4roJVgLLlmWDiMrDDMFJuSr+ZaMnxWid3WOKI5vbcq2bOLS+y2hnwyKWIaM9JEWV3YMmwmy24Jv60dLMVx8xzBn0pvFBmHDG61bAQiE0rs3qrdRjBdS30ZIvCz4o0GmRdbEn6jOkmtnjkuntcq5fhpb3zIU8vrQ47aLBYM4GyTV8YWjwSzfD9B+BNZe8thwcE4OL04gC/eMJ8Fc+ep"
"YE3UEp6A3cw1GZcVF38ZhQfqfXodvFOuuv9YcXHw4XT00MZ7S9vJesnW2hw1nglRMm91paGQR+JiCI5Q+Z2xWNKbRJRe+b/RZ7gY1+Lw0jo7PGZmPAFTrBa2pUQPMnE6j+sLY/2FoR9YpnUN5jEmFQZK+qnx7aULRfSrAvT9bntcpntSxt1WzMvAygAneAojiV27vSb2K7XUj3G4pUqdJD1N2rLIPCXWcqyuB3f85QvpxHAskXcZoC7y9Wn/S1KGQRuvsLn9l30IR5J/lImUh4v3YOHy8JZM6G1Jd83iVT//jd8nw="
"1; mode=block"
"nosniff"
"SAMEORIGIN"
"1.1 vegur"
"DYNAMIC"
"max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""
"cloudflare"
"5548ccbc1f9cd8a1-CPH"
Traceback (most recent call last):
	7: from -e:1:in `<main>'
	6: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/chainable.rb:20:in `get'
	5: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/client.rb:34:in `request'
	4: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/redirector.rb:61:in `perform'
	3: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/redirector.rb:98:in `redirect_to'
	2: from /usr/local/bundle/gems/http-5.0.0.pre/lib/http/request.rb:106:in `redirect'
	1: from /usr/local/lib/ruby/2.6.0/forwardable.rb:230:in `join'
/usr/local/bundle/gems/addressable-2.7.0/lib/addressable/uri.rb:1884:in `join': Can't convert Array into String. (TypeError)
@dentarg
Copy link
Contributor Author

dentarg commented Jan 13, 2020

This looks like a bug in the HTTP parser used (https://github.com/tmm1/http_parser.rb), but I thought I would first report it here. (I haven't yet taken the time to reproduce it with only http_parser.rb.)

Ah, maybe it is the expected behaviour of the parser, headers["Location"] return an Array. Maybe the code in HTTP should be able to work with that.

root@8b3b9a934e9b:/app# irb -rhttp
irb(main):001:0> HTTP.get("https://burd.se/httprb2").headers["Location"].class
=> Array
irb(main):002:0> HTTP.get("https://burd.se/httprb2").headers["Location"].size
=> 3

@ixti
Copy link
Member

ixti commented Jan 13, 2020

We don't use tmm1/http_parser.rb any more. We switched to ffi-based http-parser: #489

It's a bug of HTTP::Redirector (which I really need to extract out of the client I believe). So, basically bug is caused by my mistake in HTTP::Headers API design. Namely HTTP::Headers#[] will return header String if there's only one header with matching name or will return an Array if more than one.

Easiest fix will be to change this line:

@request = redirect_to @response.headers[Headers::LOCATION]

To something like:

@request  = redirect_to @response.headers.get(Headers::LOCATION).first

Alternatively we can fail in this case (with a bit more meaningful message though) when multiple Location headers per RFC 7320 Section 3.2.2 which allows only Set-Cookie to be presented multiple times.

See Also

@ixti ixti added the Bug label Jan 13, 2020
@dentarg
Copy link
Contributor Author

dentarg commented Jan 13, 2020

@ixti the problem isn't multiple location headers, there's only one for https://burd.se/httprb2, it's just that it's very long :) It works in both Google Chrome and curl FYI

@dentarg
Copy link
Contributor Author

dentarg commented Jan 13, 2020

I should correct myself, I only see one Location header with Chrome and curl, but maybe they work around it?

@ixti
Copy link
Member

ixti commented Jan 13, 2020

@dentarg Hmm. Seems like their logic is actually:

@response.headers.get(Headers::LOCATION).join

But server is definitely sending 3 Location headers:

require "http"
require "logger"
HTTP.use(:logging => { :logger => Logger.new(STDOUT) }).get("https://burd.se/httprb2")

You will see something like this:

I, [2020-01-13T19:44:44.632679 #208992]  INFO -- : > GET https://burd.se/httprb2
D, [2020-01-13T19:44:44.632724 #208992] DEBUG -- : Connection: close
Host: burd.se
User-Agent: http.rb/4.3.0


I, [2020-01-13T19:44:45.783447 #208992]  INFO -- : < 302 Found
D, [2020-01-13T19:44:45.783801 #208992] DEBUG -- : Date: Mon, 13 Jan 2020 18:44:45 GMT
Content-Type: text/html;charset=utf-8
Transfer-Encoding: chunked
Connection: close
Set-Cookie: __cfduid=df7aa2ed71abace1946c845bfc23c09751578941084; expires=Wed, 12-Feb-20 18:44:44 GMT; path=/; domain=.burd.se; HttpOnly; SameSite=Lax
Location: https://itty.bitty.site/#/?XQAAAAKhFgAAAAAAAAAeGQknlxhIQakTrqORUwLuwNEJhII5BpfZZe5tdT5QIwTSvOsK2m3Bfw5s9+y2qRxSY1BcMjsN6DS6cL01kzF+/htSy2Bl4kCeWPe2MR3DiTiQ+y3SwAXN7Cxy+UVIJS6RNKq2PuLesCU9Q+7WTJVZ0Av/eNbp5ES4W/mXBdTuS/ePyhh5VlUKyMKkTqakZqYXgeS9YtMYinYS5oQhdkYruc4Bv2D3r621D2i5JheDlF7kTQ3uZSTtoWW+y8/ox2KYVZP8j5YOLwJTNQSLYdZ8im1nJynRN0UhzX0ztHlae7rDee5gTzByVPXv+c6Xd7UHSsdhMdgY2WSG67yQ1Zs1jPGX3daqF5VGETN1aUwCJbKa1ke7yxhlMdatQ9p8t3V4ISbW4DPyStoGlk87MfFyqpZOqL/nzUWOeHqHeb77VP/GQn83m9phRjBBkImblZSdIKAMJE4+K/Df4YWH0zlor7WAnO1dwS8hXYkY5FiCSCPKZ888EQgOeQhf3tBPe9NK7HX54cl2BN4AzBFjSRPnGFEa3cbD6cm2zmmpDoMfF+BD0H9Xv9JTegg8kG8T+erwf8I7/lvs9AKpFa2dyT/Pn2RNM2ZUHgLnhmbAKpms/lWmk5HZ355T3HSue4WSvnd8VFGxvRg1fnzh4+uqT3GeYHpak0XRE4FXtdvnNml3WUVER/GeAfeiXOw2UGXDuneHfhaLPam9xuVaL1YX54fwVQ/iWBjPzc9ZBFO5AKm2BbqCmBykyBYQL9mrO6AHG2TqxbAULXb3YOFfITtHTokI7lmr7Vi6iA6uj+3LWdcHEieXgdR42tAOetFjcxnkg2O8ebYgs0kO+aklrZP8/V6Q1UE858rNDz9ejavwc20k+bh5UwL7Ls4VSOk3Yqvrk3wG1aMsOA6V9vxk6nyPLtR8oBXkbhE5ElHiCYd1qd6f94JiRUWcqgfCSNDYes8pWifn9tj9YE2ZygFJ2e2l8mmMcnSQ0m1oXEGYnzY9dnBGEmL9cHYzppASVxner
Location: AYHjVVPcUX8l6Zh8txIrDwWJU8RKjBwYRYq0yYW9X3bP5gumSWRPT7rConR1ulDEHhrHiF2WbF366uigOO/dHffQYtxCOA0ep5wy4nhuHYnPLuCsc/aeaAktAWZXOiQ8W/obYr9Ng52dfmILf6X8o56O8h5vvhD21NOzRsTVCiA/l7qMutRhB87GYP5kQ/DlMy27u8xeQ8pip4rPwinxaWDx5BkSIMTcFHHa5B0Xn9PUlUBXeugCuE2Onqf34SMgp3vxMEsIZBeU942SBysVYrmvPFS0T1pwZU8qiwIX4H8T7kxGzyp9CcomeFnWJb24slRzSglUV9LkyAQ4NibfOBwFQCcp58YSTMtYVytkQGV2a3OxivDXxPdewmrM7mx4X9Ezvz5xO2u0a/h2nMBRGk1xeOwLH14CTOTN1MBbYr+zQysRBxB+xtHIIxhBz/UdoZeVnIwGltv79NbukWn1+J+56Aafch3PQ6HVuoYG2t1ZUYOxLOr42fRwMKuEWIZ+D1zRIcn0jUVBv7I2NT3F6FmcHMGE/Mop3zGKSQ/J4R9Lg1DUgHAdzs6ikdgNXderLCnK/r9VBuR+k35hp0jPcs9B6mWPH7Fg4uoxZDgT438nDMj1F/gn222+vMKjf6x1vQlcB9OqzCabhVzAhAV3Q187RO/fNtVBJ+K/j2I+Bsa1dWFuWsmknieGZ7vdaJne0BcO1KJ+A3VkvC2DAsJlcR9PABdBgQesNzMONmXTkLkPwP0FWrQ1g2Wk6poeBX3NVKkN0/uNwq4hsXOf5ITiO5ZnOeGXF+LPQ9OKh8jXeaqBnRYXAHI2qHmqavNv7qLtygi8tFBIMjPHG3y4bfX3IhGhQFdreYLx/Rclse1Ohkp/FzI+5YVsTc7t6aegSHvRUsZX7KcroTvOT/Bbr0b/JUbuacrh3esQ+OsxWNGtImSe/YJHl7Hgdjz3VONC2QauizMswXT77ITbwl5xc7RBJ0eDKgKhoBdLrJfyOuhDxuWV7xn4R2Z86PgfQ3OfE6ngbaH4MswD/+NK3e6Ss48LHWI/tmrk6z16O4S5UztIt6IeaQvwn2jxN1qbI7KrZk9QZ1ZseLv4ELjbbU0inAJqzIYXxV3rQoMFWasBKTI/csjhnpMCmbQHTjeJvl+TECNqsj4Ay/D4roJVgLLlmWDiMrDDMFJuSr+ZaMnxWid3WOKI5vbcq2bOLS+y2hnwyKWIaM9JEWV3YMmwmy24Jv60dLMVx8xzBn0pvFBmHDG61bAQiE0rs3qrdRjBdS30ZIvCz4o0GmRdbEn6jOkmtnjkuntcq5fhpb3zIU8vrQ47aLBYM4GyTV8YWjwSzfD9B+BNZe8thwcE4OL04gC/eMJ8Fc+ep
Location: YE3UEp6A3cw1GZcVF38ZhQfqfXodvFOuuv9YcXHw4XT00MZ7S9vJesnW2hw1nglRMm91paGQR+JiCI5Q+Z2xWNKbRJRe+b/RZ7gY1+Lw0jo7PGZmPAFTrBa2pUQPMnE6j+sLY/2FoR9YpnUN5jEmFQZK+qnx7aULRfSrAvT9bntcpntSxt1WzMvAygAneAojiV27vSb2K7XUj3G4pUqdJD1N2rLIPCXWcqyuB3f85QvpxHAskXcZoC7y9Wn/S1KGQRuvsLn9l30IR5J/lImUh4v3YOHy8JZM6G1Jd83iVT//jd8nw=
X-Xss-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
Via: 1.1 vegur
Cf-Cache-Status: DYNAMIC
Expect-Ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
Server: cloudflare
Cf-Ray: 554985f43de2a849-CDG

@dentarg
Copy link
Contributor Author

dentarg commented Jan 13, 2020

Thanks for checking that, I guess it makes sense, I assume there's some limit on how big HTTP headers are allowed to be. Either joining them or raising an HTTP::-something error is fine for my use-case.

@ixti
Copy link
Member

ixti commented Jan 13, 2020

I tend to follow what Firefox, curl, and Chrome are doing - and will simply concatenate those.

ixti added a commit that referenced this issue Jan 14, 2020
@ixti ixti closed this as completed in #587 Jan 14, 2020
ixti added a commit that referenced this issue Jan 14, 2020
ixti added a commit that referenced this issue Mar 25, 2020
@tarcieri tarcieri mentioned this issue May 13, 2021
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