search

vsTerminus / Mojo-Discord

Perl Modules that implement parts of the Discord API. Intended for Text Chat Bots.
MIT License
33 stars 10 forks source link

Mojo::UserAgent >= 9.07 breaks empty POST and PUT requests #37

Closed incognico closed 3 years ago

incognico commented 3 years ago

From the Mojo changelog:

9.07  2021-03-11
  - Improved Mojo::UserAgent performance slightly by not including unnecessary "Content-Length: 0" request headers.

This breaks most functions relying on POST or PUT without a payload like start_typing() or create_reaction() because the discord cloudflare endpoint does not like a missing Content-Length:

$VAR1 = '<!DOCTYPE html>
[...]
<title>Error 411 (Length Required)!!1</title>
[...]
  <p><b>411.</b> <ins>That's an error.</ins>
  <p>POST requests require a <code>Content-length</code> header.  <ins>Thatâs all we know.</ins>
[...]

The Content-Length: 0 header needs to be added back explicitly in Mojo-Discord for empty POST/PUT requests.

vsTerminus commented 3 years ago

Adding the header explicitly doesn't fix the issue :( Seems like Mojo::UserAgent is stripping it out regardless.

On the plus side, sending a single character in the body to get a Content-Length of 1 works and Discord accepts it.

I think the ultimate fix here is for Discord to only require Content-Length when it is >= 1

incognico commented 3 years ago

Not sure if Discord can change the requirement of Content-Length, it is probably a Cloudflare thing. Removing it when explicitly setting it does not sound good either, I'll open a Mojo issue then.

incognico commented 3 years ago

Fixed in https://github.com/mojolicious/mojo/issues/1738 so probably no action needed here, leaving the issue open until a new Mojo release is out and I can confirm that it works again,

vsTerminus commented 3 years ago

Great news. Thanks for doing that!

incognico commented 3 years ago

Fixed (and confirmed) in Mojo::UserAgent 9.11, so the workaround is needed for Mojo::UserAgent versions < 9.11 and >= 9.07. Maybe add a condition to enable the workaround if any of the affected version is used, giving there is an easy way to check the mojo version.

incognico commented 3 years ago

77b97a1ad145cf45599c36ec756c30cb45614fc0 Could be made conditional with something like:

use version;
require Mojolicious;
my $mojobroken;
$mojobroken = 1 if ( version->parse( $Mojolicious::VERSION ) >= version->parse( 9.07 ) && version->parse( $Mojolicious::VERSION ) < version->parse( 9.11 ) );
vsTerminus commented 3 years ago

I'll probably just put it in the cpanfile to be incompatible with those versions.

vsTerminus commented 3 years ago

I'm going to just leave the workaround in place. Sending a single character doesn't (currently) break Mojo::UserAgent. I can always revisit this if that changes.