Closing brace missing in webhook JSON

bug
#1

I set up a webhook in Discourse today for the first time. I’ve got a great use case for it and can’t wait to finish the code. :slight_smile:

But the post and topic event JSON sent by Discourse is not playing well with my receiving PHP script. PHP fails to parse it. Through json_last_error_msg, I saw that there is a syntax error somewhere in the payload, which is why json_decode is failing.

From the webhook’s log panel in Discourse, the request JSON validated when I tested it, so I set up a simple script to verify that PHP is receiving the same data that Discourse is reporting, but that doesn’t seem to be the case.

<?php

$data = file_get_contents('php://input');
print_r($data);

?>

Discourse receives a response that seems to show that a closing brace is omitted in the payload being received by PHP.

Here’s the well-formed JSON payload reported by Discourse:

http://pastebin.com/VTtgLWm9

…and here’s what print_r returns to Discourse:

http://pastebin.com/qCcyusu9

1 Like
Super impressed with Discourse's development cycle
#2

Any ideas @fantasticfears?

#3

PHP respects Content-Length correctly so it truncates the string in the middle. The problem is that the payload is not count in octets.

Pull request: FIX: Content-Lenght should be the size in octets by fantasticfears · Pull Request #4451 · discourse/discourse · GitHub

4 Likes
#4

@fantasticfears Can you check if Excon sets the Content-Length header for us?

#5

Yes, it does if the field is not provided. In connection.rb:

body = datum[:body].is_a?(String) ? StringIO.new(datum[:body]) : datum[:body]

# The HTTP spec isn't clear on it, but specifically, GET requests don't usually send bodies;
# if they don't, sending Content-Length:0 can cause issues.
unless datum[:method].to_s.casecmp('GET') == 0 && body.nil?
  unless datum[:headers].has_key?('Content-Length')
    datum[:headers]['Content-Length'] = detect_content_length(body)
  end
end

def detect_content_length(body)
  if body.respond_to?(:size)
    # IO object: File, Tempfile, StringIO, etc.
    body.size
  elsif body.respond_to?(:stat)
    # for 1.8.7 where file does not have size
    body.stat.size
  else
    0
  end
end

Since we need to record that field in the log, it doesn’t matter.

But it’s worthwhile to build a module for the job and redeliver action. 1) Reuse the code. 2) Easier to replace Excon.

#6

A post was split to a new topic: Super impressed with Discourse’s development cycle

#7