mschindler83 / fints-hbci-php

A PHP library to communicate with FinTS / HBCI Servers
MIT License
130 stars 61 forks source link

Parsing of the transaction description: Missing information #40

Open bell430 opened 7 years ago

bell430 commented 7 years ago

Hi @mschindler83,

first of all: Great work! I really love this tool.

I used it with comdirect bank and found an issue with the MT940-parser. I some cases, the reponse contains a divider-string within the description section, although it is still the same transaction. As a result there are parts of the name and/or description missing. Here an example:

RAW-Response (divided and personal information taken out):

Array ( [0] => [1] => 20:MT940-1701250150 [2] => 21:NONREF [3] => 25:20041144/418311700EUR [4] => 28C:0/1 [5] => 60F:C170123EUR1921,44 [6] => 61:1701230123DR139,63NMSCNONREF//POS 3393590253 [7] => 86:820?20ÃœBERTRAG/ÃœBERWEISUNG?21RECHNUNGSNR. 17/27?22END-TO-END-REF. [8] => ?23NICHT ANGEGEBEN?24Ref. J9217021C1831775/2?30GENODEF1HTR?31DExxxxxxxxxxx?32DR. MEIER [9] => 61:1701230123DR69,58NMSCNONREF//POS 3394043541 [10] => 62M:C170124EUR1199,64 )

The information from index 8 is completely lost, as the parser expects it to be part of index 7.

My workaround is as follows:

MT940.php:

protected function parseToArray()
    {
....
    $transaction = substr($day[$i], 3);
    $description = substr($day[$i + 1], 3);
    //WORKAROUND:
    if (substr($day[$i+2],0,1)=="?" && substr($day[$i+2],2,1)!=":") {
            //add missing line
            $description.=$day[$i+2];
    }

As I'm not a php-expert, I'm pretty sure that you can find a much more flexible and elegant solutions for this issue. I just wanted to show my workaround for better unterstanding. Also I'm not aware about problems which could arise from this code, when using it with other banks.

Thanks and best regards!

Edit: It's even worse. The parser has a problem with the divider-string that comdirect returns. Comdirect is sometimes sending "\r\n" in the middle of a description. If then a "-" is accidently right behind it (as it belongs to the description), the parser threats it as a new transaction.

pvonauf commented 7 years ago

Hi,

I referred to this issue in issue #23 . As I am not a php expert either, I think there is a better solution to this than mine. I got it to work in the meantime (not sure if it always works).

I still however have an issue with two arrays being created accross one single transaction. This leadsto` an incomplete description1. Now I found a way to solve this by replacing following line in line 59 of MT940.php

Original line: $days = explode($divider . '-', $this->rawData);

New line: $days=preg_split('/([\r\n][-][\r\n]|[\r\n][-][+])/', $this->rawData);

The old line does not work because my raw data (Deutsche Bank) looks like this sometimes (replaced some stuff): :86:105?20EREF+LA-12431-2017-01-02-11?21-27-02-8?22MREF+N1/13002/14/2 -H?23CRED+DExxZZZxxxxxxxxxxx?24SVWZ+01.17 Autom. Sollstell?25ung 1/3002/14/2, DG li, Stp?26l. 24 xxxxxxxxxxxx 16?27ABWA+WEG Fr xxxxxxxxx. 16 W?30BYLADEM1001?31DE00000000000000999999?32WEG xxx xxxxxxxxx. 16 Xxxxxx?33n

The point is the \r\n- in this case. Here it continues with the description "H?23CRED+DExxZZZxxxxxxxxxxx?2 ...". In other cases I saw it would either be only "-" or "-+...". But I'm not sure if these will be the only cases?

My new line does not consider @@ as a separator - didn't get it to work with the $divider variable :-(

Philipp91 commented 7 years ago

This can potentially be solved by the structured description. I added code to actually expose it here: https://github.com/mschindler83/fints-hbci-php/pull/48

mgrundkoetter commented 7 years ago

The Problem is, that there may be lines starting with "-", so the line $days = explode($divider . '-', $this->rawData); will cut the rest apart. I have a workaround for this: $days = preg_split('%^-$%', $this->rawData);, for which I will create a PR.

dsentker commented 6 years ago

The workaround from @mgrundkoetter has helped me, too. Thank you for that. It seems that this fix is not part of v1.0.4. Is there a reason why the fix has not yet been adopted?