Merge pull request 'Fix #185: Abgeschnittene PayPal-NVP-Antworten im Abrufprotokoll' (#186) from fds2610-bot/Steuerkram:main into main

Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/186

Changes.md

@@ -2,6 +2,13 @@
 
 ---
 
+## 2026-08-18 – PayPal-Abruf: abgeschnittene NVP-Antworten im Protokoll (#185)
+
+- `PayPalNvpFetchService::externalIdExists()` dedupliziert bereits über `external_id` = die PayPal-Transaktionsnummer (`L_TRANSACTIONID`), nicht über Datum/Betrag/Text – eine nachträglich korrigierte Buchung wird also schon heute nicht doppelt importiert. Kein Änderungsbedarf an diesem Teil des Issues.
+- `nvpPost()` behandelte einen abgebrochenen/unvollständigen HTTP-Transfer bisher wie einen normalen Fehler und protokollierte nur den HTTP-Status – teils `0` ohne weitere Erklärung, ohne dass sichtbar wurde, dass die Antwort abgeschnitten war.
+- **`src/Services/PayPalNvpFetchService.php`:** `nvpPost()` wertet jetzt zusätzlich `curl_errno()`/`curl_error()` sowie `CURLINFO_CONTENT_LENGTH_DOWNLOAD` aus. Neue Methode `isTruncatedTransferError()` erkennt abgebrochene Transfers (u.a. `CURLE_PARTIAL_FILE`, `CURLE_GOT_NOTHING`, `CURLE_RECV_ERROR`, sowie die Fehlermeldungen „transfer closed“/„Empty reply“) und schreibt dafür einen erklärenden Hinweis ins Abrufprotokoll statt der bisherigen stillen HTTP-Status-Zeile. Neue Methode `isLengthMismatch()` erkennt zusätzlich den Fall, dass trotz HTTP 200 weniger Bytes ankamen als im `Content-Length`-Header angekündigt, und meldet das ebenfalls als Hinweis.
+- **`tests/91_PayPalNvpAbgeschnitteneAntwortTest.php`** (neu): Deckt `isTruncatedTransferError()` (bekannte cURL-Fehlercodes, Fehlermeldungstexte, normale Fehler ohne Falsch-Alarm) und `isLengthMismatch()` (kürzere Antwort, übereinstimmende Länge, fehlende Content-Length) über Reflection auf die privaten statischen Methoden ab – ohne echte PayPal-Verbindung.
+
 ## 2026-08-12 – SSRF-Schutz für SMTP-Einstellungen (#183)
 
 - `mail_smtp_host`/`mail_smtp_port` wurden ungeprüft aus `Setting::get()` an `SmtpMailer::connect()` durchgereicht, das per `stream_socket_client()` eine TCP-/TLS-Verbindung zu genau diesem Ziel aufbaut – anders als `fints_url` (#125) und `nextcloud_url` (#127) ohne jede Prüfung auf private/interne Adressen. Über `POST /settings/mail-test` konnte ein authentifizierter Nutzer damit interne Netzwerk-/Portscans auslösen (Blind-SSRF-Oracle, CWE-918), da unterschiedliche `RuntimeException`-Nachrichten offene/geschlossene/gefilterte Ports bzw. Fremddienste erkennbar machten.

src/Services/PayPalNvpFetchService.php

@@ -278,17 +278,53 @@ class PayPalNvpFetchService
         ]);
         $response    = curl_exec($ch);
         $code        = curl_getinfo($ch, CURLINFO_HTTP_CODE);
+        $errNo       = curl_errno($ch);
+        $errMsg      = curl_error($ch);
+        $declaredLen = (float)curl_getinfo($ch, CURLINFO_CONTENT_LENGTH_DOWNLOAD);
         curl_close($ch);
 
-        if ($code !== 200 || !$response) {
+        if ($response === false) {
+            if (self::isTruncatedTransferError($errNo, $errMsg)) {
+                ($this->log)('Hinweis: Antwort vom PayPal-Server wurde abgeschnitten (' . $errMsg . ').');
+            } else {
+                ($this->log)('HTTP-Status: ' . $code . ($errMsg !== '' ? ' (' . $errMsg . ')' : ''));
+            }
+            return null;
+        }
+
+        if (self::isLengthMismatch($response, $declaredLen)) {
+            ($this->log)(sprintf(
+                'Hinweis: Antwort scheint abgeschnitten zu sein (%d von %d Bytes empfangen).',
+                strlen($response),
+                (int)$declaredLen
+            ));
+        }
+
+        if ($code !== 200 || $response === '') {
             ($this->log)('HTTP-Status: ' . $code);
             return null;
         }
 
-        parse_str((string)$response, $result);
+        parse_str($response, $result);
         return $result;
     }
 
+    /** cURL meldet einen abgebrochenen/unvollständigen Transfer nicht immer über denselben Fehlercode. */
+    private static function isTruncatedTransferError(int $errNo, string $errMsg): bool
+    {
+        if (in_array($errNo, [CURLE_PARTIAL_FILE, CURLE_GOT_NOTHING, CURLE_RECV_ERROR], true)) {
+            return true;
+        }
+        return stripos($errMsg, 'transfer closed') !== false
+            || stripos($errMsg, 'Empty reply') !== false;
+    }
+
+    /** Erkennt eine Antwort, die trotz HTTP 200 kürzer ist als vom Server angekündigt. */
+    private static function isLengthMismatch(string $response, float $declaredLength): bool
+    {
+        return $declaredLength > 0 && strlen($response) < (int)$declaredLength;
+    }
+
     private function testCredentials(): bool
     {
         $result = $this->nvpPost(['METHOD' => 'GetBalance']);

tests/91_PayPalNvpAbgeschnitteneAntwortTest.php

@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+use Steuerkram\Services\PayPalNvpFetchService;
+
+/**
+ * Test 91: PayPalNvpFetchService – Erkennung abgeschnittener Antworten (#185).
+ *
+ * Eine abgeschnittene NVP-Antwort darf nicht still verworfen werden, sondern
+ * muss als Hinweis im Abrufprotokoll landen. Deckt die beiden dafür
+ * zuständigen Hilfsmethoden isTruncatedTransferError() und isLengthMismatch()
+ * ab, die ohne echte PayPal-Verbindung testbar sind.
+ */
+class PayPalNvpAbgeschnitteneAntwortTest extends TestCase
+{
+    protected string $testName = 'Test 91: PayPalNvpFetchService – abgeschnittene Antworten';
+
+    public function run(): void
+    {
+        $this->testIsTruncatedTransferErrorErkenntBekannteFehlercodes();
+        $this->testIsTruncatedTransferErrorErkenntFehlermeldungstext();
+        $this->testIsTruncatedTransferErrorIgnoriertNormaleFehler();
+        $this->testIsLengthMismatchErkenntKuerzereAntwort();
+        $this->testIsLengthMismatchOhneFehlalarm();
+    }
+
+    private function callPrivateStatic(string $method, array $args = []): mixed
+    {
+        $ref = new \ReflectionMethod(PayPalNvpFetchService::class, $method);
+        $ref->setAccessible(true);
+        return $ref->invokeArgs(null, $args);
+    }
+
+    private function testIsTruncatedTransferErrorErkenntBekannteFehlercodes(): void
+    {
+        foreach ([CURLE_PARTIAL_FILE, CURLE_GOT_NOTHING, CURLE_RECV_ERROR] as $errNo) {
+            $this->assert(
+                $this->callPrivateStatic('isTruncatedTransferError', [$errNo, '']) === true,
+                "isTruncatedTransferError() erkennt cURL-Fehlercode {$errNo} als abgeschnittenen Transfer"
+            );
+        }
+    }
+
+    private function testIsTruncatedTransferErrorErkenntFehlermeldungstext(): void
+    {
+        $this->assert(
+            $this->callPrivateStatic('isTruncatedTransferError', [
+                0,
+                'transfer closed with 128 bytes remaining to read',
+            ]) === true,
+            'isTruncatedTransferError() erkennt "transfer closed"-Meldungen auch bei unbekanntem Fehlercode'
+        );
+        $this->assert(
+            $this->callPrivateStatic('isTruncatedTransferError', [0, 'Empty reply from server']) === true,
+            'isTruncatedTransferError() erkennt "Empty reply from server"'
+        );
+    }
+
+    private function testIsTruncatedTransferErrorIgnoriertNormaleFehler(): void
+    {
+        $this->assert(
+            $this->callPrivateStatic('isTruncatedTransferError', [0, '']) === false,
+            'isTruncatedTransferError() meldet ohne Fehlercode/-text keinen Abbruch'
+        );
+        $this->assert(
+            $this->callPrivateStatic('isTruncatedTransferError', [CURLE_COULDNT_CONNECT, 'Connection refused']) === false,
+            'isTruncatedTransferError() stuft einen Verbindungsfehler nicht als abgeschnittene Antwort ein'
+        );
+    }
+
+    private function testIsLengthMismatchErkenntKuerzereAntwort(): void
+    {
+        $this->assert(
+            $this->callPrivateStatic('isLengthMismatch', ['ACK=Success', 100.0]) === true,
+            'isLengthMismatch() erkennt eine Antwort, die kürzer als Content-Length ist'
+        );
+    }
+
+    private function testIsLengthMismatchOhneFehlalarm(): void
+    {
+        $vollstaendig = 'ACK=Success';
+        $this->assert(
+            $this->callPrivateStatic('isLengthMismatch', [$vollstaendig, (float)strlen($vollstaendig)]) === false,
+            'isLengthMismatch() schlägt bei übereinstimmender Länge nicht an'
+        );
+        $this->assert(
+            $this->callPrivateStatic('isLengthMismatch', ['ACK=Success', 0.0]) === false,
+            'isLengthMismatch() schlägt ohne bekannte Content-Length (0/-1) nicht an'
+        );
+        $this->assert(
+            $this->callPrivateStatic('isLengthMismatch', ['ACK=Success', -1.0]) === false,
+            'isLengthMismatch() schlägt bei negativer Content-Length nicht an'
+        );
+    }
+}