Project

General

Profile

Actions

Bug #22464

closed

Foreman attempts to parse & errors on empty body

Added by James Shewey about 6 years ago. Updated almost 5 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Difficulty:
Triaged:
Yes
Fixed in Releases:
Found in Releases:

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.


Related issues 1 (0 open1 closed)

Related to Infoblox - Bug #22487: Plugin does not cause smart-proxy to return a bodyRejectedActions
Actions #1

Updated by James Shewey about 6 years ago

  • Related to Bug #22487: Plugin does not cause smart-proxy to return a body added
Actions #2

Updated by The Foreman Bot about 6 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/5235 added
Actions #3

Updated by The Foreman Bot about 6 years ago

  • Pull request https://github.com/theforeman/foreman/pull/5236 added
Actions #4

Updated by James Shewey about 6 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

Actions #5

Updated by The Foreman Bot over 5 years ago

  • Status changed from Ready For Testing to New
  • Pull request deleted (https://github.com/theforeman/foreman/pull/5236)
Actions #6

Updated by Lukas Zapletal almost 5 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.

Actions

Also available in: Atom PDF