Merge pull request #554 from rest-client/ab-warn-content-type

Add warnings and docs to avoid payload header foot gun.

README.md

@@ -601,6 +601,14 @@ Basic `x-www-form-urlencoded` POST params:
     "url"=>"https://httpbin.org/post"}
 ```
 
+JSON payload: rest-client does not speak JSON natively, so serialize your
+payload to a string before passing it to rest-client.
+```ruby
+>> payload = {'name' => 'newrepo', 'description': 'A new repo'}
+>> RestClient.post('https://api.github.com/user/repos', payload.to_json, content_type: :json)
+=> <RestClient::Response 201 "{\"id\":75149...">
+```
+
 Advanced GET params (arrays):
 ```ruby
 >> r = RestClient.get('https://http-params.herokuapp.com/get', params: {foo: [1,2,3]})

lib/restclient/request.rb

@@ -432,7 +432,26 @@ module RestClient
     #
     def make_headers(user_headers)
       headers = stringify_headers(default_headers).merge(stringify_headers(user_headers))
-      headers.merge!(@payload.headers) if @payload
+
+      # override headers from the payload (e.g. Content-Type, Content-Length)
+      if @payload
+        payload_headers = @payload.headers
+
+        # Warn the user if we override any headers that were previously
+        # present. This usually indicates that rest-client was passed
+        # conflicting information, e.g. if it was asked to render a payload as
+        # x-www-form-urlencoded but a Content-Type application/json was
+        # also supplied by the user.
+        payload_headers.each_pair do |key, val|
+          if headers.include?(key)
+            warn("warning: Overriding #{key.inspect} header " +
+                 "#{headers.fetch(key).inspect} with #{val.inspect} " +
+                 "due to payload")
+          end
+        end
+
+        headers.merge!(payload_headers)
+      end
 
       # merge in cookies
       cookies = make_cookie_header

spec/unit/request_spec.rb

@@ -260,6 +260,34 @@ describe RestClient::Request, :include_helpers do
     end
   end
 
+  it 'warns when overriding existing headers via payload' do
+    expect(fake_stderr {
+      RestClient::Request.new(method: :post, url: 'example.com',
+                              payload: {'foo' => 1}, headers: {content_type: :json})
+    }).to match(/warning: Overriding "Content-Type" header/i)
+    expect(fake_stderr {
+      RestClient::Request.new(method: :post, url: 'example.com',
+                              payload: {'foo' => 1}, headers: {'Content-Type' => 'application/json'})
+    }).to match(/warning: Overriding "Content-Type" header/i)
+
+    expect(fake_stderr {
+      RestClient::Request.new(method: :post, url: 'example.com',
+                              payload: '123456', headers: {content_length: '20'})
+    }).to match(/warning: Overriding "Content-Length" header/i)
+    expect(fake_stderr {
+      RestClient::Request.new(method: :post, url: 'example.com',
+                              payload: '123456', headers: {'Content-Length' => '20'})
+    }).to match(/warning: Overriding "Content-Length" header/i)
+  end
+
+  it 'does not warn for a normal looking payload' do
+    expect(fake_stderr {
+      RestClient::Request.new(method: :post, url: 'example.com', payload: 'payload')
+      RestClient::Request.new(method: :post, url: 'example.com', payload: 'payload', headers: {content_type: :json})
+      RestClient::Request.new(method: :post, url: 'example.com', payload: {'foo' => 'bar'})
+    }).to eq ''
+  end
+
   it "uses netrc credentials" do
     expect(Netrc).to receive(:read).and_return('example.com' => ['a', 'b'])
     request = RestClient::Request.new(:method => :put, :url => 'http://example.com/', :payload => 'payload')