Skip to content

fix(response): avoid error if response is too long#246

Open
benjamin-si wants to merge 1 commit into
ebics-api:3.xfrom
benjamin-si:3.x
Open

fix(response): avoid error if response is too long#246
benjamin-si wants to merge 1 commit into
ebics-api:3.xfrom
benjamin-si:3.x

Conversation

@benjamin-si

Copy link
Copy Markdown

fix(response): avoid error if response is too long / truncated

Problem

When the EBICS server returns a very long (in my case Large response size: 1051214 bytes, in ebics 2.4 for Société générale bank) or truncated response, the XML body can become malformed. This currently leads to two cascading issues:

  1. Silent XML parse failureHttpClient::post() uses @$response->loadXML($contents), which suppresses the error and returns false. The code continues with an empty/unloaded Response object, making debugging difficult.
  2. Type errors downstreamResponseHandler extracts the transaction key via retrieveH00XTransactionKey(), which returns null when the expected node is missing. The code then immediately calls base64Service->decode(null) on a string-typed parameter, causing a TypeError or later null-pointer crashes inside Segment / DownloadSegment where transactionKey is declared as non-nullable string.

Solution

This PR adds defensive handling at every layer so that truncated responses fail early and explicitly instead of propagating cryptic type errors.

Layer Change
HTTP (HttpClient.php) Remove the @ suppression on loadXML. If parsing fails, throw a clear RuntimeException: "Failed to parse EBICS XML response. Response may be truncated or malformed."
Initialization (ResponseHandler.php) In extractInitializationSegment(), check whether TransactionKey is present. If it is null, throw immediately with: "EBICS initialization segment is missing TransactionKey. Response may be truncated."
Download (ResponseHandler.php) In extractDownloadSegment(), allow transactionKeyEncoded to be null. Pass null to the segment instead of crashing on base64_decode(null).
Models (Segment.php, DownloadSegment.php) Widen the type of $transactionKey from string to ?string (and update getters/setters) so that a missing key is representable without violating type safety.


$response = new Response();
$response->loadXML($contents);
$loaded = @$response->loadXML($contents);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. But I really do not like supression for functions. I think better try, catch or something like error handler. So see real message in line 53.

{
$transactionKeyEncoded = $this->retrieveH00XTransactionKey($response);

if (null === $transactionKeyEncoded) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transaction key for some requests is not present, but for these requests extractInitializationSegment() should not be applied. This is more wrong usage exception here. I would say, that transactionKeyEncoded is required in this place and can be thrown appropriate exception if no transactionKeyEncoded.

}

public function setTransactionKey(string $transactionKey): void
public function setTransactionKey(?string $transactionKey): void

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that it should be possible have not transactionKey for DownloadSegment.

Comment thread src/Models/Segment.php
}

public function setTransactionKey(string $transactionKey): void
public function setTransactionKey(?string $transactionKey): void

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not happen

@andrew-svirin

Copy link
Copy Markdown
Member

I've limited the maximum file size to 10MB because processing files of this size through AES, Base64, and Zip operations requires loading large amounts of data into memory, which can exceed PHP's memory limit.
The library does not include chunked processors that can handle larger files by processing them in smaller portions - those are available only in the Premium version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants