moov-io / bai2

BAI2 file format is a cash management balance reporting standard
Apache License 2.0
15 stars 9 forks source link

Support strings with \ #114

Closed omerlh closed 3 months ago

omerlh commented 3 months ago

Bai2 Version: latest, compled from main

What were you trying to do? I have BAI2 (actually BAI3) file from my bank and it fails validation on lines with string containing \ which is end of line of BAI2. For example - dates like 13/05/24.

On a side note, I got this parser to read my file by changing the version on the first line of the file...

What did you expect to see? File parsed correctly

What did you see? Errors like:

Error: ERROR parsing account on line 218 (unable to read record type 04)

How can we reproduce the problem? Create a BAI2 file with a text containing \

Thanks!

adamdecaf commented 3 months ago

Is this the same issue as https://github.com/moov-io/bai2/issues/113 ?

wokkaflokka commented 3 months ago

@adamdecaf they seem similar.

I had encountered the same behavior and noted it here in the moov slack.

I finally ran into a file with data like this in production, and have been motivated to take another pass at addressing this. PR incoming.

wokkaflokka commented 3 months ago

@omerlh would it be possible for you to provide a sample file that highlights the failure and is representative of the data you need to parse? I have a proposed fix here and could use this test case to ensure it covers your scenario.

omerlh commented 3 months ago

Sorry for not looking into open issues! Yes, it seems like the same issue...

@wokkaflokka uploading a file will be challenging but I am happy to test a branch locally if you want :)

omerlh commented 3 months ago

I just tested the latest code on main and it still failing with the same error for this line:

88,<redacted>,<redacted>=/<redacted>/<redacted>
wokkaflokka commented 3 months ago

@omerlh I understand that difficulty.

Would it be possible to take a file and scrub it or extract a relevant piece of it? The snippet you post is useful -- if you could create a representative file, even if it's only 1 transaction, it would help to write a test around.

Is the 88 record a continuation for a transaction or for an account?

omerlh commented 3 months ago

Sorry - my bad! I just tested it and it parses my files correctly :) Thanks for the quick fix!

adamdecaf commented 3 months ago

Fixed in https://github.com/moov-io/bai2/pull/116

wokkaflokka commented 3 months ago

Would it be fine to cut a new release? v.0.4.0 (or v.0.3.1)?

adamdecaf commented 3 months ago

Yea, I'll make a release.