Rocketchat webhook payload without "payload="

[joconcepts]

SUMMARY

Rocketchat webhook does not need to have payload={{data}} as payload but only data directly as payload.

bugfixes:
  - rocketchat: fix webhook payload

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

rocketchat

ADDITIONAL INFORMATION

# before the payload was like this:
"payload={\"text\": \"Test\", \"channel\": \"#test\", \"username\": \"me\", \"icon_url\": \"https://docs.ansible.com/favicon.ico\", \"link_names\": 1}"}

# but this results in a 400 bad request
"body": "{\"success\":false,\"error\":\"Unexpected token 'p', \\\"payload{\\\"t\\\"... is not valid JSON\"}"

# after this change the payload is like that
"{\"text\": \"Test\", \"channel\": \"#test\", \"username\": \"me\", \"icon_url\": \"https://docs.ansible.com/favicon.ico\", \"link_names\": 1}"

[ansibullbot]

cc @Deepakkothandan
click here for bot help

[felixfontein]

Since this has been part of the module for many years (basically since its inception in 2016: ansible/ansible@20ea765), I guess that RocketChat's behavior changed over time.

Since RocketChat can be self-hosted, this looks like a breaking change to me that needs some mechanism so it also works with older versions of RocketChat.

Pinging the last person to contribute (a64df65) to this module, @mmslkr, who might be still using it as well.

[felixfontein]

Another PR with the same change came up: #9882

I looked a bit at Rocket.Chat, and my guess is that RocketChat/Rocket.Chat@263a7cb caused this change. That commit ended up in Rocket.Chat 7.4.0 (and all its release candidates).

Looking at https://github.com/RocketChat/Rocket.Chat/pull/34999/files#diff-ed9bdef3b65986a40ddc3a005e5b490e513df772ca3398c8f55f8e0460d90760R245, I think the current syntax is still fine once the content-type header is set to application/x-www-form-urlencoded. (My guess is that before the content-type header would have to be set to JSON without the payload=, but 🤷)

Can someone try to change the module to instead send the content-type header with value application/x-www-form-urlencoded and see whether that works? I think that would be the less dangerous patch.

For that, the line response, info = fetch_url(module, rocketchat_incoming_webhook, data=payload) needs to be changed to response, info = fetch_url(module, rocketchat_incoming_webhook, data=payload, headers={'content-type': 'application/x-www-form-urlencoded'}) or something like that.

[broferek]

@felixfontein I tried making it work with your suggestions around adding the header. But I couldn't end up with a working solution.

Instead, I made a new commit in my branch which basically uses the new syntax by default and falls back to the old one if it failed.

I assume that it doubles the number of queries but we can assume the majority of Rocket Chat users will upgrade to at least Rocket Chat 7.4.0 in the coming year as per Rocket Chat deprecating version policy, therefore the fallback should not be used anymore or by very few people by then.

[felixfontein]

It looks like they removed the code that handles that in RocketChat/Rocket.Chat#35183. I have no idea why it's called a regression; it should rather be called a breaking change and mentioned in the changelog.

[russoz]

#9882 is kinda moving faster. May I suggest we consolidate our efforts in that PR? (And close this one)