4642: Slack notification fails due to incorrect Content-Type header

splatter2

What version of Djblets are you using?

1.0.2

Which module(s) have the problem?

rbintegrations

What steps will reproduce the problem?

  1. Create a Slack notification
  2. Submit a new review request.

What is the expected output? What do you see instead?

Slack notification should be successful, but fails. If logging is turned on, a stackrace will appear in the logs.

What version of Python and Django?

Python 2.7, DJango 1.11.6

Please provide any additional information below.

Capturing network traffic using tcpdump revealed that the Content-Type header was being set incorrectly. The Content-Type header is set to application/x-www-form-urlencoded, but the body is json. Therefore, the request is refused by Slack (or Mattermost in my case).

The culprit is at line 189 of integration.py. urllib2 defaults to form data when posting. The fix is very simple. Change the line to read:

urlopen(Request(webhook_url, 'payload='+json.dumps(payload)))

Another option is to do this instead, as explained here:

json_data = json.dumps(payload)
urlopen(Request(webhook_url, json_data, {'Content-Type': 'application/json', 'Content-Length': len(data)}))
chipx86
#1 chipx86

Despite the current code being wrong, I suspect this is only affecting Mattermost, as we use Slack every day and it's working correctly without any API requests being refused.

It's worth noting that we're building out explicit Mattermost support, maintained separately from the Slack support. At the time of initial testing, this wasn't hitting any problems, so I'm guessing either Mattermost has gotten more strict (and broke bug-for-bug compatibility with Slack), or has since gotten less strict since the version you're running.

Certainly worth fixing this properly, either way.

  • -New
    +Confirmed
  • +Release-3.0.x
  • +Component:Integration
    +EasyFix
david
#2 david

Fixed in rbintegrations master (b0667c7). This will ship in rbintegrations 1.0 (which will also include first-class Mattermost support).

  • -Confirmed
    +Fixed