Skip to content

Add json content-type for empty requests (fix net/http warning in strict mode)#5615

Closed
kolen wants to merge 1 commit intoSeleniumHQ:masterfrom
kolen:remote-http-empty-requests-add-content-type
Closed

Add json content-type for empty requests (fix net/http warning in strict mode)#5615
kolen wants to merge 1 commit intoSeleniumHQ:masterfrom
kolen:remote-http-empty-requests-add-content-type

Conversation

@kolen
Copy link
Copy Markdown

@kolen kolen commented Mar 14, 2018

When doing empty requests (for example for element click), Content-Type header is not set. This causes Net::HTTP to issue warning in $VERBOSE mode (-w):

net/http: warning: Content-Type did not set; using application/x-www-form-urlencoded

This warning shows for each action (click, etc), so there are lots of these warnings in output when running tests.

This pull request adds Content-Type: application/json to such requests too (they has {} body so I guess it's empty json "object")

Example script that shows this warning:

#!/usr/bin/env ruby -w

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "selenium-webdriver"
end

require "selenium-webdriver"

options = Selenium::WebDriver::Chrome::Options.new
options.add_argument("--headless")
driver = Selenium::WebDriver.for :chrome, options: options
driver.navigate.to "http://google.com"

element = nil
Selenium::WebDriver::Wait.new(timeout: 10)
                         .until { element = driver.find_element(tag_name: "a") }
STDERR.puts "clicking"
element.click
STDERR.puts "clicked"

I didn't add test for warning, only added check for header being added to request. warn is not easily stubbable, common way to catch warnings is to redirect stderr to StringIO which is too hacky I think.

When doing empty requests (for example for element click),
Content-Type header is not set. This causes Net::HTTP warning to show:

net/http: warning: Content-Type did not set; using
application/x-www-form-urlencoded

This sets application/json content-type for such requests (empty
requests has empty json object in body).
@jleyba jleyba added the C-rb Ruby Bindings label Mar 19, 2018
@JasonLunn
Copy link
Copy Markdown

#5659 is alternative solution to the same root problem. In that PR, the issue is resolved by doing explicitly what net-http has been doing implicitly all along, which is to supply the default content type of application/x-www-form-urlencoded if nothing was provided. This approach in this PR does seem more correct, but it isn't clear to me if there is a hard guarantee that all POSTS will be JSON, or if there are any bad behaviors on the part of the servers that might expect this default content type.

@kolen
Copy link
Copy Markdown
Author

kolen commented Mar 22, 2018

@JasonLunn Yes, I think DEFAULT_HEADERS is a good place to put content-type, if it does not break anything, and if Content-Type with empty body (including GET) is ok by standards (I can't find anything about it).

@p0deje
Copy link
Copy Markdown
Member

p0deje commented Mar 23, 2018

Thank you for the issue report and PR addressing the problem. I've decided to push a fix by myself so that it complies to JSON Wire Protocol specification of payload. This also makes Ruby bindings behave similarly to Python and Java ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants