nahi / httpclient

'httpclient' gives something like the functionality of libwww-perl (LWP) in Ruby.
https://github.com/nahi/httpclient
698 stars 287 forks source link

version 2.2.1 broke post requests #60

Closed naquad closed 12 years ago

naquad commented 12 years ago

Hi.

I've had requests called like that:

client.post("http://example.com/script", {:x => 1, :y => 'some string'}, {'X-Requested-With' => 'XMLHttpRequest'})

after update all of them got broken. Using debugger I've found out that problem is in HTTPClient::request method. It has following line:

query, body, header, follow_redirect = keyword_argument(args, :query, :body, :header, :follow_redirect)

But there's no query argument passed by HTTPClient::post method, so query gets body params :( My fix is the following:

class HTTPClientFix < HTTPClient
  def post(uri, *args, &block)
    request(:post, uri, nil, *args, &block)
  end
end

Now query is explicitly set to nil and everything is ok, POST requests are sent as they should.

nahi commented 12 years ago

I think HTTPClient and HTTPClientFix are generating the same request. Am I missing something?

% cat tst.rb
require 'httpclient'

class HTTPClientFix < HTTPClient
  def post(uri, *args, &block)
    request(:post, uri, nil, *args, &block)
  end
end

client1 = HTTPClient.new
client2 = HTTPClientFix.new

[client1, client2].each do |client|
  client.debug_dev = STDERR
  client.test_loopback_http_response << "HTTP/1.0 200 OK\r\n\r\nOK\n"
  client.post("http://example.com/script", {:x => 1, :y => 'some string'}, {'X-Requested-With' => 'XMLHttpRequest'})
end
% ruby tst.rb
= Request

! CONNECT TO example.com:80
! CONNECTION ESTABLISHED
POST /script HTTP/1.1
X-Requested-With: XMLHttpRequest
Content-Type: application/x-www-form-urlencoded
Date: Wed, 12 Oct 2011 11:11:19 GMT
Content-Length: 17
Host: example.com

x=1&y=some+string

= Response

HTTP/1.0 200 OK

OK
! CONNECTION CLOSED
= Request

! CONNECT TO example.com:80
! CONNECTION ESTABLISHED
POST /script HTTP/1.1
X-Requested-With: XMLHttpRequest
Content-Type: application/x-www-form-urlencoded
Date: Wed, 12 Oct 2011 11:11:19 GMT
Content-Length: 17
Host: example.com

x=1&y=some+string

= Response

HTTP/1.0 200 OK

OK
! CONNECTION CLOSED
% 
naquad commented 12 years ago

in my case with 2.2.1 w/o fix post request is with body in url.

nahi commented 12 years ago

Hmm, post passes a Hash as args so it should work as a :body, not a :query.

def post(uri, *args, &block)
  request(:post, uri, argument_to_hash(args, :body, :header, :follow_redirect), &block)
end

Please show me the typescript of above tst.rb on your env.

naquad commented 12 years ago

[000][23:03:10][J:1][naquad@nq:~/projects/tests]$ cat tst.rb 
require 'httpclient'

class HTTPClientFix < HTTPClient
  def post(uri, *args, &block)
    request(:post, uri, nil, *args, &block)
  end
end

client1 = HTTPClient.new
client2 = HTTPClientFix.new

[client1, client2].each do |client|
  client.debug_dev = STDERR
  client.test_loopback_http_response << "HTTP/1.0 200 OK\r\n\r\nOK\n"
  client.post("http://example.com/script", {:x => 1, :y => 'some string'}, {'X-Requested-With' => 'XMLHttpRequest'})
end
[000][23:03:52][J:1][naquad@nq:~/projects/tests]$ ruby tst.rb 
= Request

! CONNECT TO example.com:80
! CONNECTION ESTABLISHED
POST /script?x=1&y=some+string HTTP/1.1
X-Requested-With: XMLHttpRequest
Content-Type: application/x-www-form-urlencoded
Date: Wed, 12 Oct 2011 20:03:55 GMT
Content-Length: 0
Host: example.com

= Response

HTTP/1.0 200 OK

OK
! CONNECTION CLOSED
= Request

! CONNECT TO example.com:80
! CONNECTION ESTABLISHED
POST /script HTTP/1.1
X-Requested-With: XMLHttpRequest
Content-Type: application/x-www-form-urlencoded
Date: Wed, 12 Oct 2011 20:03:55 GMT
Content-Length: 17
Host: example.com

x=1&y=some+string

= Response

HTTP/1.0 200 OK

OK
! CONNECTION CLOSED

what version of ruby do you use? I've got MRI ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-linux]

nahi commented 12 years ago

I tried 1.9.3, 1.9.2 and 1.8.7 from development repo. I've also tried 1.9.2p290 but it works as the same (parameters are in body, not in query)

ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-linux]

I uninstalled all httpclient gems and re-install 'gem install httpclient' but it still work as I expected.

Cannot imagine why it behaves so weird in your env. I should be missing something...

naquad commented 12 years ago

let me try to reinstall it too. i've got couple of gigs of gems and whole bunch of versions, maybe thats the issue.

naquad commented 12 years ago

That helped. I did full remove of /usr/lib/ruby/gems/1.9.1/{gems,doc,cache,specifications} and then did gem install httpclient -v 2.2.1. everything works ok, sorry for bothering. I don't know what was the issue, but now everything is ok.

nahi commented 12 years ago

Good to hear you solved the problem. But uninstalling (or rm -rf) should not fix the issue. If it fixes, you found the bug in ruby or rubygems. As a committer of CRuby, I wanna know the reason if I can...

I uninstalled httpclient gems in my environment because I sometimes install development versions as a local gem. So I made sure that I installed the version 2.2.1 which is the same as you.

Is it possible that you were using 2.2.0? That version has a problem around argument handling and I pushed the fix at 2.2.1 .

naquad commented 12 years ago

no, it was 2.2.1 (at least thats what gem list told me and bundler has exact version spec)

nahi commented 12 years ago

Hmm. then it might be an issue of gemset... (by rvm or by setting GEM_HOME or by using --user-install)