imagecashletter
imagecashletter copied to clipboard
Double check handling of Go zero values
Due to the nature of Go's zero values, x9 files created from JSON can end up with 0
s and empty time.Time{}
values in them. We need to ensure that we're not accidentally writing zero values in x9 files where a field should actually be empty.
ANSI X9.100-187:
All fields that are conditional and are not used shall be filled with Blanks
Bundle Control (Type 70)
-
MICRValidTotalAmount
: Can update logic to replace "0" with " " -
CreditTotalIndicator
: Valid values are 0, 1. Need to be able to replace with empty space.
Cash Letter Control Record (Type 90)
-
SettlementDate
: Should check IsZero() for time.Time values and render as empty space if true -
CreditTotalIndicator
: Valid values are 0, 1. Need to be able to replace with empty space.
Check Detail Record (Type 25)
-
MICRValidIndicator
: Valid values are 1-4. Can update logic to replace 0 with " " because 0 isn't a valid value. -
CorrectionIndicator
: Valid values are 0-4.
Check Detail Addendum A Record (Type 26)
-
BOFDCorrectionIndicator
: Valid values are 0-4.
Check Detail Addendum C Record (Type 28)
-
EndorsingBankCorrectionIndicator
: Valid values are 0-4.
File Control Record (Type 99)
-
CreditTotalIndicator
: Valid values are 0, 1. Need to be able to replace with empty space.
Image View Analysis Record (Type 54)
-
PartialImage
: Valid values are 0-2. Need to be able to replace with empty space. -
ExcessiveImageSkew
: Valid values are 0-2. Need to be able to replace with empty space. -
PiggybackImage
: Valid values are 0-2. Need to be able to replace with empty space. -
TooLightOrTooDark
: Valid values are 0-2. Need to be able to replace with empty space. -
StreaksAndOrBands
: Valid values are 0-2. Need to be able to replace with empty space. -
BelowMinimumImageSize
: Valid values are 0-2. Need to be able to replace with empty space. -
ExceedsMaximumImageSize
: Valid values are 0-2. Need to be able to replace with empty space. -
ImageEnabledPOD
: Valid values are 0-2. Need to be able to replace with empty space. -
SourceDocumentBad
: Valid values are 0-2. Need to be able to replace with empty space. -
DateUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
PayeeUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
ConvenienceAmountUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
AmountInWordsUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
SignatureUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
PayorNameAddressUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
MICRLineUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
MemoLineUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
PayorBankNameAddressUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
PayeeEndorsementUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
BOFDEndorsementUsability
: Valid values are 0-2. Need to be able to replace with empty space. -
TransitEndorsementUsability
: Valid values are 0-2. Need to be able to replace with empty space.
Image View Data Record (Type 52)
-
ClippingOrigin
: Valid values are 0-4. Need to be able to replace with empty space.
Image View Detail Record (Type 50)
-
DigitalSignatureIndicator
: 0 or 1. Need to be able to replace with empty space. -
SecurityKeySize
: 00001-99999. Can render as blank space if value = 0. -
ProtectedDataStart
: 0000000–9999999. Need to be able to replace with empty space. -
ProtectedDataLength
: 0000000–9999999. Need to be able to replace with empty space. -
ImageRecreateIndicator
: 0 or 1. Need to be able to replace with empty space.
Return Record (Type 31)
-
TimesReturned
: 0-3. Need to be able to replace with empty space.
Return Addendum A Record (Type 32)
-
BOFDCorrectionIndicator
: 0-4. Need to be able to replace with empty space.
Return Addendum B Record (Type 33)
-
PayorBankBusinessDate
: Check if value IsZero() and replace with empty space if so.
Return Addendum D Record (Type 35)
-
EndorsingBankCorrectionIndicator
: 0-4. Need to be able to replace with empty space. -
EndorsingBankIdentifier
: 0-3. Need to be able to replace with empty space.
User Record (Type 68)–Format Type 001 Payee Endorsement Record
-
EndorsementDate
: Check if value IsZero() and replace with empty space if so. -
Time
: Check if value IsZero() and replace with empty space if so. -
EndorsementIndicator
: 0-9. Need to be able to replace with empty space.
For the MICRValidIndicator
field in the Check Detail Record, this is particularly important as X9 ICL files received from the FRB will specify this field as blank.
@intricate We can probably handle that field pretty simply. One of the reasons I haven't started working on this issue is that fixing all of these fields will require breaking changes. I've been debating whether we should use pointers to primitive types (i.e. *int
instead of int
) so that we can treat nil
as empty, or possibly create a wrapper similar to SQL's NullInt64
(https://pkg.go.dev/database/sql#NullInt64)
For MICRValidIndicator
, however, we should be able to handle it without a breaking change.
@atonks2 Daniel, is there any progress on this? We are trying out this library for the first time and looks really good, but we are currently having to comment out some checks in order to verify our testing. Thanks
@smithbk Unfortunately I haven't been able to make progress on this yet. Would you mind sharing which checks you're having to disable? Perhaps with such a list we'd be able to prioritize specific fields and tackle this in multiple smaller PRs.
@atonks2 Daniel, here are the checks I've had to comment out for now.
In imageViewDetail.go:
if ivDetail.DigitalSignatureMethod != "" {
if err := ivDetail.isDigitalSignatureMethod(ivDetail.DigitalSignatureMethod); err != nil {
return &FieldError{FieldName: "DigitalSignatureMethod",
Value: ivDetail.DigitalSignatureMethod, Msg: err.Error()}
}
}
if ivDetail.ImageCreatorRoutingNumberField() == "000000000" {
return &FieldError{FieldName: "ImageCreatorRoutingNumber",
Value: ivDetail.ImageCreatorRoutingNumber,
Msg: msgFieldInclusion + ", did you use ImageViewDetail()?"}
}
if ivDetail.ImageCreatorDate.IsZero() {
return &FieldError{FieldName: "ImageCreatorDate",
Value: ivDetail.ImageCreatorDate.String(),
Msg: msgFieldInclusion + ", did you use ImageViewDetail()?"}
}
And in ReturnDetailAddendumA.go:
if rdAddendumA.BOFDEndorsementDate.IsZero() {
return &FieldError{FieldName: "BOFDEndorsementDate",
Value: rdAddendumA.BOFDEndorsementDate.String(),
Msg: msgFieldInclusion + ", 5. did you use ReturnDetailAddendumA()?"}
}
Thanks, Keith
@atonks2 Daniel, could you provide an estimate of when this might be fixed? Thanks
Hey @smithbk . I'm afraid I don't have an estimate on this one. Updating ICL to better handle default vs empty values is almost certainly going to be a breaking change and I haven't had time to give it the consideration it needs to minimize that impact.
I started digging into the fields you mentioned and I don't believe they relate to this issue. The scope of this issue is optional fields, which are erroneously flagged as invalid because of Go's default (aka "zero") values. For example you may have a file where MICRValidIndicator
is omitted/empty (which is valid), but that file in this library would end up with Go's default int
value for this field (0
), which is not a valid MICRValidIndicator
value.
For the fields/checks you mentioned:
Digital Signature Method
The specification says this is conditional, and our library only performs validation on this field if a value is present. If DigitalSignatureMethod
is empty, the check is skipped.
Image Creator Routing Number
This is defined as a mandatory field in ANSI-X9.100-187, so our library returns an error if the field is missing/invalid.
Image Creator Date
This is also a mandatory field, so omitting it will result in an error.
BOFD/Endorsement Date
Also a mandatory field.
If any of the above is a misunderstanding on my part, or you have a version of the specification that differs in these field definitions from ANSI-X9.100-187 (2016), lets open new issues to address those concerns.
@atonks2 Daniel, in order to not introduce any breaking changes, I'm curious if you've considered (and if it would work) to add "HasXXX" bool fields for each of these problematic fields. For example, for MICRValidTotalAmount
, add a field named HasMICRValidTotalAmount
which is a bool for someone to use to distinguish between the MICRValidTotalAmount field being non-existent and the field existing with a zero value? Just a thought.
I appreciate the suggestion @smithbk. That would work for people importing this codebase as a library to construct files, but would not work for those using the API to upload raw x9 files.
The two options I have in mind each come with their own pros and cons:
- Use pointers to primitive types so fields that are intentionally absent would be
nil
(e.g. use*int
instead ofint
) - Use or implement a wrapper around the primitive types, like this sql package.