Fix HTTP header trailers for cURL

[Alkarex]

fix #942

Some feeds are not parsed correctly due to wrong handling of HTTP trailer headers such as Server-Timing.

Example of feed:
https://www.caranddriver.com/rss/all.xml
(Note that the bug may or may not exhibit itself depending on the cURL and PHP versions)

Downstream discussion:

• [Feed problem] "Invalid document end" for caranddriver.com FreshRSS/FreshRSS#7981
• Fix SimplePie support of HTTP trailer headers FreshRSS/FreshRSS#7983

SimplePie tries to XML parse something like </rss>server-timing: rtt; dur=6.131, retrans; dur=0, trailer-timestamp; dur=1758222994925 which obviously fails.

In the case of normal HTTP transfer (not chunked), we need to use content-length to know where the body stops, but content-length is wrong if any compression was used.
So I believe we should let cURL perform the separation of HTTP headers and body instead of using the SimplePie parser.

[jtojnar]

So I believe we should let cURL perform the separation of HTTP headers and body instead of using the SimplePie parser.

That sounds reasonable. cURL is good HTTP client, we should not replicate its functionality ourselves.

Following the breadcrumbs unfortunately does not tell us much about the rationale for introducing it:

• 4b1fc33 introduced Parser::prepareHeaders
• e5b93e1 extracted parsing to SimplePie_HTTP_Parser
• Initially introduced in 5bf1814, too large to look into the reasoning.

---

In the case of normal HTTP transfer (not chunked), we need to use content-length to know where the body stops

HTTP 1

In non-chunked HTTP response (version 1), there are no trailers, stuff after Content-Length should be either ignored, or if re-using the connection for successive requests, be parsed as part of the next message.

In chunked response, Content-Length appears to be not used. In fact one of the reasons to use chunked encoding is that the length of the whole content is not yet available because the content is streamed in chunks.

Inspecting the Reassembled TLS in Wireshark produced by env SSLKEYLOGFILE=/tmp/sslkey.log curl --http1.1 https://www.caranddriver.com/rss/all.xml, they indeed not send Content-Length with HTTP 1, and trailers are inserted into last chunk (declared with 0 length). Actually, it seems that the trailer headers are only terminated by the second CRLF of the 0 chunk there is no explicitly specified length that I can see.

HTTP 1.1 chunked response with trailer

CRLF denoted with C-style escape sequences, with actual new lines inserted after each CRLF for readability.

HTTP/1.1 200 OK\r\n
Connection: keep-alive\r\n
Content-Type: text/xml; charset=utf-8\r\n
x-robots-tag: all\r\n
Accept-Ranges: bytes\r\n
Age: 24\r\n
Date: Sun, 21 Sep 2025 09:35:43 GMT\r\n
Set-Cookie: _pxhd=7870bf6de03134b9ca13753fe377f14fe48fec3ce80d764681c3e9045efe3267:57ec8275-96ce-11f0-b3c9-2072c9786893; Max-Age=31536000; path=/; SameSite=Lax\r\n
X-Cache: HIT, MISS\r\n
Vary: Accept-Encoding\r\n
Server-Timing: time-start-msec;dur=1758447343064,time-elapsed;dur=180,fastly-pop;desc=VIE,hit-state;desc=MISS-CLUSTER\r\n
set-cookie: location_data={"country_code":"CZ","postal_code":"436 01"}; path=/;\r\n
Link: <https://googletagservices.com>; rel=preconnect, <https://api.backfires.caranddriver.com>; rel=preconnect, <https://securepubads.g.doubleclick.net>; rel=preconnect, <https://cdn.optimizely.com>; rel=preconnect, <https://adservice.google.com>; rel=preconnect, <https://connect.facebook.net>; rel=preconnect, <https://logx.optimizely.com>; rel=preconnect, <https://www.facebook.com>; rel=preconnect, <https://h.nexac.com>; rel=preconnect, <https://gtrk.s3.amazonaws.com>; rel=preconnect, <https://graph.facebook.com>; rel=preconnect, <https://hips.hearstapps.com>; rel=preconnect, <https://nexus.ensighten.com>; rel=preconnect, <https://www.google-analytics.com>; rel=preconnect, <https://www.googletagmanager.com>; rel=preconnect, <https://stats.g.doubleclick.net>; rel=preconnect, <https://www.google.com>; rel=preconnect, <https://tpc.googlesyndication.com>; rel=preconnect, <https://fonts.googleapis.com>; rel=preconnect, <https://img.vast.com>; rel=preconnect, <https://z.moatads.com>; rel=preconnect\r\n
strict-transport-security: max-age=31557600; includeSubDomains\r\n
x-country: CZ\r\n
Cache-Control: max-age=0, must-revalidate, private\r\n
alt-svc: h3=":443";ma=86400,h3-29=":443";ma=86400,h3-27=":443";ma=86400\r\n
transfer-encoding: chunked\r\n
trailer: server-timing\r\n
\r\n
32c6\r\n
<?xml version="1.0" encoding="UTF-8"?>\n<rss version="2.0" xmlns:atom="http://www.w3.org/2005/Atom" xmlns:media="http://search.yahoo.com/mrss/">\n…\n</rss>\r\n
0\r\n
server-timing: rtt; dur=19.315, retrans; dur=0, trailer-timestamp; dur=1758447343245\r\n
\r\n

For completeness, here is also gzipped version captured from SSLKEYLOGFILE=/tmp/sslkey.log curl --compressed --http1.1 https://www.caranddriver.com/rss/all.xml:

The same request but compressed

HTTP/1.1 200 OK\r\n
Connection: keep-alive\r\n
Content-Type: text/xml; charset=utf-8\r\n
Content-Encoding: gzip\r\n
x-robots-tag: all\r\n
Accept-Ranges: bytes\r\n
Age: 32\r\n
Date: Sun, 21 Sep 2025 10:18:34 GMT\r\n
Set-Cookie: _pxhd=820a4553273c294ccaed386f4dce2a532a4f730eaea4f9f465b2783cdcca392b:544d3522-96d4-11f0-a86e-a41b8f8b85a3; Max-Age=31536000; path=/; SameSite=Lax\r\n
X-Cache: HIT, MISS\r\n
Vary: Accept-Encoding\r\n
Server-Timing: time-start-msec;dur=1758449913956,time-elapsed;dur=181,fastly-pop;desc=VIE,hit-state;desc=MISS-CLUSTER\r\n
set-cookie: location_data={"country_code":"CZ","postal_code":"436 01"}; path=/;\r\n
Link: <https://googletagservices.com>; rel=preconnect, <https://api.backfires.caranddriver.com>; rel=preconnect, <https://securepubads.g.doubleclick.net>; rel=preconnect, <https://cdn.optimizely.com>; rel=preconnect, <https://adservice.google.com>; rel=preconnect, <https://connect.facebook.net>; rel=preconnect, <https://logx.optimizely.com>; rel=preconnect, <https://www.facebook.com>; rel=preconnect, <https://h.nexac.com>; rel=preconnect, <https://gtrk.s3.amazonaws.com>; rel=preconnect, <https://graph.facebook.com>; rel=preconnect, <https://hips.hearstapps.com>; rel=preconnect, <https://nexus.ensighten.com>; rel=preconnect, <https://www.google-analytics.com>; rel=preconnect, <https://www.googletagmanager.com>; rel=preconnect, <https://stats.g.doubleclick.net>; rel=preconnect, <https://www.google.com>; rel=preconnect, <https://tpc.googlesyndication.com>; rel=preconnect, <https://fonts.googleapis.com>; rel=preconnect, <https://img.vast.com>; rel=preconnect, <https://z.moatads.com>; rel=preconnect\r\n
strict-transport-security: max-age=31557600; includeSubDomains\r\n
x-country: CZ\r\n
Cache-Control: max-age=0, must-revalidate, private\r\n
alt-svc: h3=":443";ma=86400,h3-29=":443";ma=86400,h3-27=":443";ma=86400\r\n
transfer-encoding: chunked\r\n
trailer: server-timing\r\n
\r\n
1eb3\r\n
[binary gzipped data]\r\n
0\r\n
server-timing: rtt; dur=24.233, retrans; dur=0, trailer-timestamp; dur=1758449914137\r\n
\r\n

HTTP 2

HTTP 2 replaces chunked encoding with custom framing so the trailers are explicitly separated into HEADERS frame.

000096DC  61 6e 6e 65 6c 3e 0a 3c  2f 72 73 73 3e 00 00 35   annel>.< /rss>..5
000096EC  01 05 00 00 00 01 7f 06  b2 b1 29 fb 52 4b 6c 80   ........ ..).RKl.
000096FC  2e 2e 20 b3 f4 a5 85 4d  83 a9 1f 6a 49 6d 90 03   .. ....M ...jIm..
0000970C  e9 44 d8 33 50 5b 16 49  a9 2a 12 3a 6b fb 52 4b   .D.3P[.I .*.:k.RK
0000971C  6c 80 2e b6 f3 4d 80 42  69 96 81                  l....M.B i..

Here is Wireshark’s breakdown of the HEADERS frame (starting with 00 00 35 on the first line above

HyperText Transfer Protocol 2
    Stream: HEADERS, Stream ID: 1, Length 53
        Length: 53
        Type: HEADERS (1)
        Flags: 0x05, End Headers, End Stream
        0... .... .... .... .... .... .... .... = Reserved: 0x0
        .000 0000 0000 0000 0000 0000 0000 0001 = Stream Identifier: 1
        [3 Body fragments (37289 bytes): #52(14574), #67(16384), #72(6331)]
        Data […]: 3c3f786d6c2076657273696f6e3d22312e302220656e636f64696e673d225554462d38223f3e0a3c7273732076657273696f6e3d22322e302220786d6c6e733a61746f6d3d22687474703a2f2f7777772e77332e6f72672f323030352f41746f6d2220786d6c6e733a6d656469613d22687
        [Pad Length: 0]
        Header Block Fragment: 7f06b2b129fb524b6c802e2e20b3f4a5854d83a91f6a496d9003e944d833505b1649a92a123a6bfb524b6c802eb6f34d8042699681
        [Header Length: 90]
        [Header Count: 1]
        Header: server-timing: rtt; dur=16.213, retrans; dur=0, trailer-timestamp; dur=1758450224340
            Name Length: 13
            Name: server-timing
            Value Length: 69
            Value: rtt; dur=16.213, retrans; dur=0, trailer-timestamp; dur=1758450224340
            [Unescaped: rtt; dur=16.213, retrans; dur=0, trailer-timestamp; dur=1758450224340]
            Representation: Literal Header Field with Incremental Indexing - Indexed Name
            Index: 69

[Alkarex]

My guess is that the parser was mainly needed for the fsockopen() mode.

[jtojnar]

It appears to have been available in PHP 4 already so it should be fine: https://www.php.net/ChangeLog-4.php#4.4.5. Very nice improvement 👍

[jtojnar]

This appears to break HTTP proxy tunnel.

Running tinyproxy -d -c tinyproxy.conf with the following config:

Port 8888
Listen 127.0.0.1
Timeout 600
Allow 127.0.0.1

The test below fails:

--- a/tests/Integration/HTTP/ClientsTest.php
+++ b/tests/Integration/HTTP/ClientsTest.php
@@ -290,4 +290,33 @@ class ClientsTest extends TestCase
             'Body does not end as expected: ' . $response->get_body_content()
         );
     }
+
+    /**
+     * Check that proxy client handling works
+     */
+    public function testProxyTunnel(): void
+    {
+        $client = new FileClient(new Registry(), [
+            'curl_options' => [
+                \CURLOPT_HTTPPROXYTUNNEL => true,
+                \CURLOPT_PROXY => '127.0.0.1',
+                \CURLOPT_PROXYPORT => 8888,
+            ]
+        ]);
+        $server = new MockWebServer();
+        $server->start();
+
+
+        $url = $server->setResponseOfPath(
+            '/hello',
+            new MockWebServerResponse('Hello World!', [], 200)
+        );
+
+        $response = $client->request(Client::METHOD_GET, $url);
+
+        $server->stop();
+
+        self::assertSame(200, $response->get_status_code());
+        self::assertSame('Hello World!', $response->get_body_content());
+    }
 }

1) SimplePie\Tests\Integration\HTTP\ClientsTest::testProxyTunnel
SimplePie\HTTP\ClientException: assert($this->body !== null)

/home/jtojnar/Projects/simplepie/src/HTTP/FileClient.php:68
/home/jtojnar/Projects/simplepie/tests/Integration/HTTP/ClientsTest.php:315

Caused by
AssertionError: assert($this->body !== null)

/home/jtojnar/Projects/simplepie/src/File.php:312
/home/jtojnar/Projects/simplepie/src/Registry.php:203
/home/jtojnar/Projects/simplepie/src/HTTP/FileClient.php:59
/home/jtojnar/Projects/simplepie/tests/Integration/HTTP/ClientsTest.php:315

[jtojnar]

Quoting https://www.rfc-editor.org/rfc/rfc9110#section-6.5.2-3 (emphasis mine):

At the end of a message, a recipient MAY treat the set of received trailer fields as a data structure of name/value pairs, similar to (but separate from) the header fields.

I guess we should just stop after the first line consisting of only CRLF, as SimplePie\HTTP\Parser does not support trailers.

Though I guess this might be problematic with tunnelling too. curl_exec return value will include the contents of HTTP tunnel so Parser::prepareHeaders expects something like this (new lines added for clarity):

HTTP/1.1 200 Connection established\r\n
Proxy-agent: tinyproxy/1.11.2\r\n
\r\n
HTTP/1.1 404 Not Found\r\n
Host: 127.0.0.1:56009\r\n
Date: Sun, 21 Sep 2025 15:00:53 GMT\r\n
Connection: close\r\n
X-Powered-By: PHP/8.0.30\r\n
Content-type: text/html; charset=UTF-8\r\n
\r\n
Hello World!

[Alkarex]

From memory, cURL returning or not the proxy headers depend on picking the correct type of proxy settings such as CURLOPT_HTTPPROXYTUNNEL.

[jtojnar]

They will be included whenever using tunnelling (unless disabled with CURLOPT_SUPPRESS_CONNECT_HEADERS (but that is only available in PHP 7.3)).

But Parser::prepareHeaders will strip it just fine, as long as the empty CRLF line is preserved.

So I would suggest using CURLOPT_WRITEFUNCTION to get the body rather than getting it from curl_exec, and ignoring any CURLOPT_HEADERFUNCTION calls after CURLOPT_WRITEFUNCTION was called.

[Alkarex]

What about using only CURLOPT_HEADERFUNCTION but keeping the blanks?
906fa67

I am worried CURLOPT_WRITEFUNCTION may decrease performance and increase memory use

[jtojnar]

Tested Transfer-encoding: chunked and looks like curl itself handles this correctly:

Test server

<?php

declare(strict_types=1);

$socket = stream_socket_server("tcp://0.0.0.0:8888", $errno, $errstr);

echo "connected\n";

if (!$socket) {
    echo "Failed to open socket: $errstr ($errno)<br />\n";
} else {
    while ($conn = stream_socket_accept($socket)) {
        echo "accepted\n";

        // echo "simulating response to CONNECT\n";
        // fwrite(
        //     $conn,
        //     // "HTTP/1.1 100 Continue\r\n"
        //     // . "\r\n"
        //     // .
        //     "HTTP/1.1 200 Connection established\r\n"
        //     . "Proxy-agent: tinyproxy/1.11.2\r\n"
        //     . "\r\n"
        // );

        sleep(1);

        fwrite(
            $conn,
            "HTTP/1.1 100 Continue\r\n"
            . "\r\n"
            . "HTTP/1.1 100 Continue\r\n"
            . "\r\n"
            . "HTTP/1.1 200 OK\r\n"
            . "Host: 127.0.0.1:56009\r\n"
            . "Date: Sun, 21 Sep 2025 15:00:53 GMT\r\n"
            . "Connection: close\r\n"
            . "X-Powered-By: PHP/8.0.30\r\n"
            . "Content-type: text/html; charset=UTF-8\r\n"
            . "Transfer-encoding: chunked\r\n"
            // . "Content-length: 12\r\n"
            . "\r\n"
            // . "Hello World!"
            . "5\r\n"
            . "Hello"
            . "\r\n6\r\n"
            . "World!"
            . "\r\n0\r\n\r\n"
        );

        sleep(1);

        fclose($conn);
    }

    fclose($socket);
}

[jtojnar]

Interestingly, this removal in fact fixes overwrite the final response code returned by CURLINFO_HTTP_CODE with the interim one (e.g. 100) when server returns an interim response. Extracted this fix into #945 with other related fixes.