Bug #22464
closedForeman attempts to parse & errors on empty body
Description
When attempting to create a DNS record using an Infoblox DNS smart proxy, the response returned does not contain a body (the below from a Packet Capture of the RestClient from the Foreman and the SmartProxy webserver):
Request headers
POST /dns/ HTTP/1.1 Accept: application/json Accept-Encoding: gzip, deflate User-Agent: rest-client/2.0.1 (linux-gnu x86_64) ruby/2.2.2p95 X-Request-Id: ca7db6e7fe4d5a1a729d9ee28e1a8cbae66cdf7571e3e03b7ec758bd98a9c9b8 Content-Length: 59 Content-Type: application/x-www-form-urlencoded Host: foreman.example.com:8000 fqdn=juana-porraz.example.com&value=192.168.1.189&type=A
Response headers
HTTP/1.1 200 OK Content-Type: text/html;charset=utf-8 Content-Length: 0 X-Xss-Protection: 1; mode=block X-Content-Type-Options: nosniff X-Frame-Options: SAMEORIGIN Server: Date: Wed, 24 Jan 2018 17:24:48 GMT Connection: Keep-Alive
This appears to be the expected behavior. On the [https://github.com/theforeman/smart_proxy_dns_infoblox/blob/master/lib/smart_proxy_dns_infoblox/dns_infoblox_main.rb smart proxy side], it looks like the results in the body of the response are probably just relayed to Foreman:
def ib_create(clazz, params) clazz.new({ :connection => connection }.merge(params)).post end
This would then be relayed to Foreman which [https://github.com/theforeman/foreman/blob/698e916ce208b5040b83a908a058c83c94d158ee/lib/proxy_api/dns.rb will attempt to parse] it:
# Sets a DNS entry # [+fqdn+] : String containing the FQDN of the host # [+args+] : Hash containing :value and :type: The :fqdn key is taken from the fqdn parameter # Returns : Boolean status def set(args) parse post(args, "") rescue => e raise ProxyException.new(url, e, N_("Unable to set DNS entry")) end
And it will attempt to parse it [using JSON.parse]:
# Decodes the JSON response if no HTTP error has been detected # If an HTTP error is received then the error message is saves into @error # Returns: Response, if the operation is GET, or true for POST, PUT and DELETE. # OR: false if a HTTP error is detected # TODO: add error message handling def parse(response) if response && response.code >= 200 && response.code < 300 return response.body.present? ? JSON.parse(response.body) : true else false end rescue => e logger.warn "Failed to parse response: #{response} -> #{e}" false end
In the case of a content-length of 0, this will cause JSON.parse to attempt to parse an empty string and [https://stackoverflow.com/questions/30621802/why-does-json-parse-fail-with-the-empty-string throw an error].
According to the RFC for JSON (RFC7231 https://tools.ietf.org/html/rfc7231#section-6.3.1) a 0 content-length is a perfectly valid JSON response however:
Aside from responses to CONNECT, a 200 response always has a payload, though an origin server MAY generate a payload body of zero length.
To prevent this condition from causing a failure, we need to check for a content-length of 0 and skip the parsing.
Updated by James Shewey over 7 years ago
- Related to Bug #22487: Plugin does not cause smart-proxy to return a body added
Updated by The Foreman Bot over 7 years ago
- Status changed from New to Ready For Testing
- Pull request https://github.com/theforeman/foreman/pull/5235 added
Updated by The Foreman Bot over 7 years ago
- Pull request https://github.com/theforeman/foreman/pull/5236 added
Updated by James Shewey over 7 years ago
I have filed an upstream bug for this issue: https://bugs.ruby-lang.org/issues/14451
And have submitted a PR: https://github.com/theforeman/foreman/pull/5236
Updated by The Foreman Bot almost 7 years ago
- Status changed from Ready For Testing to New
- Pull request deleted (
https://github.com/theforeman/foreman/pull/5236)
Updated by Lukas Zapletal almost 6 years ago
- Status changed from New to Rejected
- Triaged changed from No to Yes
Hello, thanks for the report. I believe that Rest Client and Ruby HTTP client stacks are responsible for handling content length. It's not that simple to rely on content length because these days HTTP servers use chunked encoding as well - we should not try to write that.
Read timeout error is totally fine if it's the server which messed up resonse - and I believe this is the case here:
https://github.com/theforeman/smart_proxy_dns_infoblox/pull/23
I am closing the ticket for now as I want to focus on fixing smart-proxy.