cjrasmussen / BlueskyApi

Simple helper for interacting with the Bluesky API/AT protocol
MIT License
15 stars 3 forks source link

Problem with older version of curl and empty CURLOPT_POSTFIELDS #11

Closed jaybeaton closed 2 months ago

jaybeaton commented 2 months ago

With an older version of curl, a call to com.atproto.server.refreshSession was always failing with a 400 Bad Request response. These were the response headers:

HTTP/1.1 400 Bad Request
Server: awselb/2.0
Date: Mon, 09 Sep 2024 15:31:33 GMT
Content-Type: text/html
Content-Length: 122
Connection: close

Some debugging showed that curl was never actually making the call to the remote server-- it was just immediately throwing that error. The issue was that curl was set to make a POST request, but CURLOPT_POSTFIELDS was not set.

Making this change fixed the issue:

diff --git a/vendor/cjrasmussen/bluesky-api/src/BlueskyApi.php b/vendor/cjrasmussen/bluesky-api/src/BlueskyApi.php
index 364f12a..5f11d5d 100644
--- a/vendor/cjrasmussen/bluesky-api/src/BlueskyApi.php
+++ b/vendor/cjrasmussen/bluesky-api/src/BlueskyApi.php
@@ -109,6 +109,7 @@ class BlueskyApi
                switch ($type) {
                        case 'POST':
                                curl_setopt($c, CURLOPT_POST, 1);
+                               curl_setopt($c, CURLOPT_POSTFIELDS, NULL);
                                break;
                        case 'GET':
                                curl_setopt($c, CURLOPT_HTTPGET, 1);

This change did not affect a POST with actual data (e.g. BlueskyApi::startNewSession()) because CURLOPT_POSTFIELDS is set later with actual values by this:

if ($body) {
    curl_setopt($c, CURLOPT_POSTFIELDS, $body);
} elseif (($type !== 'GET') && (count($args))) {
    curl_setopt($c, CURLOPT_POSTFIELDS, json_encode($args, JSON_THROW_ON_ERROR));
}

I'm not sure what exactly what versions of curl will have problems, but this is the version on my server that was failing (from 2013!):

$ curl --version
curl 7.29.0 (x86_64-redhat-linux-gnu) libcurl/7.29.0 NSS/3.90 zlib/1.2.7 libidn/1.28 libssh2/1.8.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smtp smtps telnet tftp
Features: AsynchDNS GSS-Negotiate IDN IPv6 Largefile NTLM NTLM_WB SSL libz unix-sockets

This version worked fine as it was:

$ curl --version
curl 7.74.0 (aarch64-unknown-linux-gnu) libcurl/7.74.0 OpenSSL/1.1.1w zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0 nghttp2/1.43.0 librtmp/2.3
Release-Date: 2020-12-09, security patched: 7.74.0-1.3+deb11u11
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli GSS-API HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets

So, it appears that you can make this change to support older versions of curl without causing any issues on newer versions.

cjrasmussen commented 2 months ago

Awesome catch. I handled it a little bit differently (in that second block you noted there's now a condition that catches if we're posting with no args) but this issue is resolved as of v2.0.4.

jaybeaton commented 2 months ago

That solution seems to work with my old curl, too. Thanks!