Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for potential issue where application is using both aws-s3 and right_http_connection #7

Open
wants to merge 1 commit into from

7 participants

@gammons

Sometimes applications are using both right_aws AND aws-s3. This is notably true with the paper_clip gem, which is relying on aws-s3.

Both right_http_connection and aws-s3 do a few tricks with Net::HTTP. My patch will rescue an error where send_request_with_body_stream is potentially redefined by aws-s3. Their version only takes 4 arguments, as opposed to your 5.

@richmeyers

A better solution probably would be to omit send_only argument if it is equal to the default. This assumes that the patches can in fact be applied in either order, which is not necessarily the case: http://rightaws.rubyforge.org/right_aws_gem_doc/files/README_txt.html, search for "‘incompatible Net::HTTP monkey-patch‘ ".

@benr75

+1

@richmeyers

I think this issue and #4 are fixable by adding a new method with 5 arguments instead of the current approach of changing the signature of an existing method to take 5 arguments instead of 4. Reading right_aws, aws-s3 and rest-client source it seems to me that all three could coexist if right_aws patched Net::HTTP more carefully.

@gammons

Oh, I had no idea that send_request_with_body_stream was a private method in Net::HTTP! Why would right_aws chagne its method signature? That seems bad! Very bad! No wonder things are breaking.

right_aws definitely needs an update. My pull request is simply a band-aid. right_aws should not be redefining method signatures of core ruby libraries.

Furthermore this pull request has been opened for months. I get the hint that this gem is no longer supported. Is it being supported elsewhere?

@richmeyers

When I said "right_aws" I meant of course right_http_connection, which is this project. right_aws uses right_http_connection to talk to aws.

right_aws (I mean it this time) does seem to be receiving more maintainer love than right_http_connection.

@gammons

ditto. I meant right_http_connection as well.

@richmeyers

I intend to take a stab at rewriting Net::HTTP patches by right_http_connection to be less intrusive, no promises on when this happens though.

@pashamur

Is there any update on this issue - it's been two years since the last comment. I'm trying to update a relatively old paperclip installation to a newer version and I ran into this issue. Solved temporarily by removing right_http_connection & right_aws altogether, but it would be nice if these gems could play nicely together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 3, 2011
  1. @gammons
This page is out of date. Refresh to see the latest.
Showing with 8 additions and 1 deletion.
  1. +8 −1 lib/net_fix.rb
View
9 lib/net_fix.rb
@@ -82,7 +82,14 @@ def exec(sock, ver, path, send_only=nil) #:nodoc: internal use only
if @body
send_request_with_body sock, ver, path, @body, send_only
elsif @body_stream
- send_request_with_body_stream sock, ver, path, @body_stream, send_only
+ begin
+ send_request_with_body_stream sock, ver, path, @body_stream, send_only
+ rescue ArgumentError
+ # this rescue is called when users have both net_http_connection and aws-s3 gem installed.
+ # For whatever reason, sometimes this method gets redefined by aws-s3 (depending on load order)
+ # the aws-s3 gem only has 4 options, not 5, so users see an ArgumentError (5 for 4) when this gets called.
+ send_request_with_body_stream sock, ver, path, @body_stream
+ end
else
write_header(sock, ver, path)
end
Something went wrong with that request. Please try again.