Fix numerous bugs, design issues, and documentation

[hydrogen18]

Hi, I have been working with this library for about a week now and have found some problems

1. HTTParty::Request#handle_deflation does not do anything useful. In actuality, decompression is performed by Net::HTTP. The Net::HTTP::GenericRequest object has a 'decode_content' value (boolean) that the documentation says is false if 'Accept-Encoding' is specified. This is done with a check in the '[]=' operator on the class, but this isn't called. HTTParty uses the 'initialize_http_header' method, which does not perform this check. So decode_content is always true. When the Net::HTTP::Response object is instantiated, this is copied over. Then Net::HTTP performs the decompression. Not only is 'handle_deflation' dead code, there is already an implementation of this in Net::HTTP

Resolution: Remove 'handle_deflation' method and calls. If a caller specifies 'Accept-Encoding' header, then explicitly set it on the object so that Net::HTTP skips decompression.

2. The monkey patch in 'lib/httparty/net_digest_auth.rb' improperly manipulates @header. The Net::HTTPHeader object requires all keys in the @header
   instance variable to be downcased, but this code adds headers with capital letters

Resolution: Call add_field as this does the correct thing without the caller having to worry about it. Use get_fields for access

3. HTTParty::Request#setup_digest_auth results in all requests being sent twice if the user specifies digest_auth. There is no way that this makes sense.

Resolution: Check for 401 and 'www-authenticate' header when considering the response. Resend the request if the user configured digest_auth and the server supports digest_auth

4. HTTParty::Request#send_authorization_headers? uses self.defined which is silly and error prone.

Resolution: set @changed_hosts to false in initialize method and use that value

5. HTTParty::Request::Headers defines the equality (==) operator without considering the downcased version of the right hand side.

Resolution: If the right hand side is a Net::HTTPHeader, compare against the @header variable of the right hand side object directly.
Use SimpleDelegator to forward methods to @header. If the right hand side is a has, convert the hash to an instance of ourselves and perform
the above comparison if the hash does not exactly equal our own @header

6. HTTParty::Request#encode_with_ruby_encoding rescues StandardError and completely suppresses the exception. In actuality, the only thing that can happen is Encoding.find may raise ArgumentError. The call to #force_encoding doesn't fail, because nothing actually happens at that time.

Resolution: Check 'Encoding.name_list.include?(charset)' before calling '#force_encoding'. No exception handling needed.

7. The documentation for ConnectionAdapter.connection is incomplete.

Resolution: List all the keys that need to be checked in the options hash by an implementation

8. ConnectionAdapter.connection does not document helper methods used by the existing implementation

Resolution: Add documentation about using clean_host

9. Trying to the method method on HTTParty::Response results in the most vexing of errors. such as stuff like this

[6] pry(main)> r.class
=> HTTParty::Response
[7] pry(main)> r.method(:qux)
NameError: undefined method `qux' for class `String'
from /home/ericu/.rvm/gems/ruby-2.3.0/gems/httparty-0.14.0/lib/httparty/response.rb:81:in `method'

This doesn't make any sort of logical sense given that I have a Response, not a string. It looks like for some reason this class inherits from BasicObject. There is a haphazard implementation of respond_to?, class, etc. It's far simpler to just define the methods needed and inherit from Object

Resolution: Switch base class to Object. Implement nil? so the object behaves like nil for an empty response. Implement to_s. Implement display .Remove un-needed implementations of other methods that Object provides.

Implement respond_to_missing? instead of respond_to?. Use super instead of RESPOND_TO_METHODS.include?(method_name)
Reference: http://blog.marc-andre.ca/2010/11/15/methodmissing-politely/

If you would like further elaboration on any of these issues or have questions just let me know.

Here is the output from running bundle exec rake:
https://gist.github.com/hydrogen18/23f812df0fd7679cb1694f772302f6ef

[jnunemaker]

Sorry for the delay. There were a lot of changes and most of them were in areas of the code base I haven't worked in so I had to do some reading. :) Really appreciate that you took the time to do all this.

[hydrogen18]

Thanks