Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TraceNumber should be int64 #236

Closed
adamdecaf opened this issue Aug 11, 2018 · 6 comments
Closed

TraceNumber should be int64 #236

adamdecaf opened this issue Aug 11, 2018 · 6 comments

Comments

@adamdecaf
Copy link
Member

A quick run of https://github.com/alecthomas/gometalinter (which can be ran for free on PR's via https://golangci.com/) gives the following error.

This would be a breaking change as int -> int64 isn't automatic.

I'm also running this on a 32-bit machine (yea, I know...), so that might be it. I thought Go defaulted int to int64 though..

$ gometalinter.v2 addenda02_test.go:23:26:error: constant 121042880000123 overflows int (vet)
addenda98_test.go:17:26:error: constant 91012980000088 overflows int (vet)
addenda98_test.go:36:29:error: constant 99912340000015 overflows int (vet)
addenda98_test.go:37:34:error: constant 99912340000015 overflows int (vet)
addenda98_test.go:45:27:error: constant 91012980000088 overflows int (vet)
addenda98_test.go:46:34:error: constant 91012980000088 overflows int (vet)
addenda99_test.go:15:28:error: constant 99912340000015 overflows int (vet)
addenda99_test.go:36:29:error: constant 99912340000015 overflows int (vet)
addenda99_test.go:37:36:error: constant 99912340000015 overflows int (vet)
addenda99_test.go:48:27:error: constant 91012980000066 overflows int (vet)
addenda99_test.go:48:27:error: too many errors (vet)
addenda02_test.go:23:26:warning: constant 121042880000123 overflows int (vetshadow)
addenda98_test.go:17:26:warning: constant 91012980000088 overflows int (vetshadow)
addenda98_test.go:36:29:warning: constant 99912340000015 overflows int (vetshadow)
addenda98_test.go:37:34:warning: constant 99912340000015 overflows int (vetshadow)
addenda98_test.go:45:27:warning: constant 91012980000088 overflows int (vetshadow)
addenda98_test.go:46:34:warning: constant 91012980000088 overflows int (vetshadow)
addenda99_test.go:15:28:warning: constant 99912340000015 overflows int (vetshadow)
addenda99_test.go:36:29:warning: constant 99912340000015 overflows int (vetshadow)
addenda99_test.go:37:36:warning: constant 99912340000015 overflows int (vetshadow)
addenda99_test.go:48:27:warning: constant 91012980000066 overflows int (vetshadow)
addenda99_test.go:48:27:warning: too many errors (vetshadow)
@jjafuller
Copy link

Due to ACH limitations you'd probably be better off capping values in the library than expanding to a different date.

@bkmoovio
Copy link
Contributor

When I run
https://github.com/alecthomas/gometalinter
GitHub
alecthomas/gometalinter
gometalinter - Concurrently run Go lint tools and normalise their output
I have the following result:
C:\Users\Owner\go\src\github.com\alecthomas\gometalinter>GoMetalinter github.com/bkmoovio/ach
....\bkmoovio\ach\batch.go:323::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\batchCTX.go:62::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\converters.go:17::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\converters.go:33::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\converters.go:44::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\iatBatch.go:339::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\reader.go:236::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:431::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:433::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:435::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:437::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:439::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:441::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:443::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\validators.go:445::warning: Errors unhandled.,LOW,HIGH (gosec)
....\bkmoovio\ach\writer.go:69::warning: Errors unhandled.,LOW,HIGH (gosec)

all from gosec

I don't receive the integer overflow, but I'm 64bit System Type x64-based PC

@bkmoovio
Copy link
Contributor

We could potentially use the parsing functions for 32 bit, Adam can you run that down, to see if it that is viable?

func (c *converters) parseNumField(r string) (s int64) {
//s, _ = strconv.Atoi(strings.TrimSpace(r))
s, _ = strconv.ParseInt(r, 10, 32)

func (c *converters) parseNumField(r string) (s uint64) {
//s, _ = strconv.Atoi(strings.TrimSpace(r))
s, _ = strconv.ParseUint(r, 10, 32)

//i64, err := strconv.ParseInt(r, 10, 32)
return s

@adamdecaf adamdecaf changed the title gometalinter reports integer overflows TraceNumber should be int64 Aug 15, 2018
@adamdecaf
Copy link
Member Author

adamdecaf commented Aug 15, 2018

From an ACH document page:

A 15-digit number in which positions 1-8 are the first eight digits of the originator's routing number, and positions 9-15 are numbers assigned in ascending order to each transaction within the Company / Batch Header Record. (80-94) 15 numeric.

From the Go docs: https://golang.org/pkg/builtin/#int

int32 is the set of all signed 32-bit integers. Range: -2147483648 through 2147483647.

Obviously this holds true then: 999999999999999 > 2^32, which means on 32-bit systems TraceNumber can't hold the required data.

Is this a problem? @wadearnold do you know if we'd ever install on a 32-bit platform? I don't think we would.

Closing. If this is a problem we can re-open this issue.

@adamdecaf
Copy link
Member Author

@wadearnold should we not allow compiling this code on 32 (and 48) bit go arch's? There's some unexpected behaviour like this.

@adamdecaf
Copy link
Member Author

#244 mentions 32-bit platforms aren't supported.

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

No branches or pull requests

3 participants