Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Regex amount parsing wrong #19

Open
oceanapplications opened this issue Jun 20, 2016 · 8 comments · May be fixed by #33
Open

Regex amount parsing wrong #19

oceanapplications opened this issue Jun 20, 2016 · 8 comments · May be fixed by #33
Assignees
Labels
bug This is a bug report issue or PR that fixes a bug
Milestone

Comments

@oceanapplications
Copy link
Contributor

The function createAmountFromStr($amountString) seems to have some bugs with strings that aren't number formatted and have one or more trailing 0s. For example, 750 will parse as 7.5 but 50 parses fine.

Here's a playground link demonstrating the issue. http://tehplayground.com/#SSeFA3M80

Thanks

@asgrim asgrim added the bug This is a bug report issue or PR that fixes a bug label Jun 20, 2016
@asgrim
Copy link
Owner

asgrim commented Jun 20, 2016

This is a BC break (even if it's preferable), and we have not nearly enough unit test coverage to explicitly show the breakages. Will need to think on this!

@oceanapplications
Copy link
Contributor Author

oceanapplications commented Jun 20, 2016

I was thinking something like this at the top of the function might solve the problem.
if (strpos($amountString, '.') === FALSE && strpos($amountString, ',') === FALSE) { $amountString .= ".00"; }

The OFX spec 2.2, page 91 http://www.ofx.net/downloads/OFX%202.2.pdf states:

Amounts that do not represent whole numbers (for example, 540.32), must include a decimal
point or comma to indicate the start of the fractional amount. Amounts should not include any punctuation
separating thousands, millions, and so forth.

Are files breaking the standard format common?

@asgrim
Copy link
Owner

asgrim commented Jun 20, 2016

They rarely conform to specs, sadly! There's lots of edge cases taken into account, hence why we could really do with some more unit tests.

@asgrim
Copy link
Owner

asgrim commented Aug 21, 2016

Note: tests showing this weird behaviour are commented with @todo now: https://github.com/asgrim/ofxparser/blob/master/tests/OfxParser/OfxTest.php#L40

@asgrim asgrim self-assigned this Aug 21, 2016
@asgrim asgrim added this to the 2.0.0 milestone Aug 23, 2016
@asgrim
Copy link
Owner

asgrim commented Aug 23, 2016

As this is a BC break, I'm going to schedule this for 2.0.

enobrev added a commit to enobrev/ofxparser that referenced this issue Dec 28, 2016
The regex in Ofx::createAmountFromStr included a "?" after the "mark"
whether it be decimal or comma.  That "?" made it match number strings
regardless of whether they had a "mark" to parse out, and grossly
changed their value.

This made amounts such as 1000 or -1000 convert to 10 or -10.  Removing
the conditional after the mark resolved this issue.  There are new tests
to show that this resolves the issue without breaking others.

Resolves asgrim#19
@enobrev enobrev linked a pull request Dec 28, 2016 that will close this issue
@hillelcoren
Copy link

We're seeing the same problem when the amount is formatted as 1.00000, it's converted to 1000 rather than 1.

@frederichoule
Copy link

Same problem over here. -699 is converted to -6.99, -2000 to -20.00

@guifabrin
Copy link

guifabrin commented May 11, 2019

Same problem here i solved with

       $handle = $this->openFile();
        while (($line = fgets($handle)) !== false) {
            $line = trim($line);
            if (starts_with($line, "<TRNAMT>") && ends_with($line, "00")){
                $line.="00";
            }
            $this->fixed .= $line."\n";
        }
        $parser = new \OfxParser\Parser();
        $ofx = $parser->loadFromString($this->fixed);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This is a bug report issue or PR that fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants