moov-io / ach

ACH implements a reader, writer, and validator for Automated Clearing House (ACH) files. The HTTP server is available in a Docker image and the Go package is available.
https://moov-io.github.io/ach/
Apache License 2.0
452 stars 150 forks source link

Parse file id modifier from file header; don't assume 'A' #1316

Closed ckbaum closed 10 months ago

ckbaum commented 10 months ago

The file ID modifier won't always be "A"; other values are sometimes used to distinguish between multiple files with otherwise identical headers.

ckbaum commented 10 months ago

@adamdecaf I'm a bit confused about the test failures I'm seeing here.

test/issues/testdata/issue702-1.ach, test/issues/testdata/issue702.ach, and test/testdata/FISERV-ZEROFILE-PIMRET825324_032720_110221.ach fixtures are all missing a file ID modifier. The rulebook is pretty clear that this character is required and must be uppercase A-Z or numeric 0-9. Not sure how these tests were passing before the recent change which hardcodes this value to A.

adamdecaf commented 10 months ago

My bad on hardcoding this as part of the optimization. We were reading it out from the file before.

I don't think we ever validated this field since it was mostly "A", but we'll start.

adamdecaf commented 10 months ago

What do you think if we revert the minor perf optimization for FileHeader's and validate FileIDModifier?

ckbaum commented 10 months ago

Either way sounds fine to me! Thanks 👍

adamdecaf commented 10 months ago

https://github.com/moov-io/ach/releases/tag/v1.33.2 has been released with this, thanks @ckbaum!