Skip to content

Fix ISSUE-141: edi reader stuck in a dead-loop with unrecognized raw segment at the end of the input. #143

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

Merged
merged 1 commit into from
Mar 28, 2021

Conversation

jf-tech
Copy link
Owner

@jf-tech jf-tech commented Mar 28, 2021

This PR fixes #141:

The triggering raw segment in the input is at line 36: "DTP":

...
L30 EB*C**42^45*MA**26*0~
L31 DTP*292*RD8*20170101-20171231~
L32 EB*B**30*MA**26*0~
L33 HSD***DA**30*0~
L34 HSD***DA**31*60~
L35 HSD*****26*1~
L36 DTP*435*RD8*20170101-20171231~
...

(added line number at beginning of each line so we can reference them)

The issue is with the schema, where inside EB seg group, we have, in this particular order, the following child segments: EB, DTP, LS, HSD. You can see the DTP seg is declared in front of HSD, but in the input it appears after. Now both DTP and HSD seg decl has min = 0. When edi reader sees the 3 HSD, it thinks it's okay, since DTP seg is optional. Then edi reader sees the DTP. Unfortunately this time it can't find a proper placement match for it, thus it considers the EB seg group is done. Cascadingly, it considers the wrapping transaction_set_id seg group as well as the GS and ISA are all done. Thus it moves on. Then it sees SE and IEA both are optional, so seems like to it everything is done. EDI reader now rewinds the seg processing stacking all the way to the #root. At this moment, essentially we have a dangling raw segment DTP unprocessed and unaccounted for. EDI reader should've gracefully failed, but it didn't. Due a bug, it will try to mark the current instance of #root seg decl done, and move onto the next instance of #root decl. But again, DTP is still not matched in the next instance of #root (the first segment in this schema should always be ISA), it will move on to the next instance of #root, yet again and again and again - infinite dead-loop.

The fix is:

If we can't match a raw seg name to the current decl stack, and if the current decl stack is nothing but the virtual #root decl, then we have a dangling unmated seg and return a failure immediately.

…segment at the end of the input.

The triggering raw segment in the input is at line 36: "DTP":

```
...
L30 EB*C**42^45*MA**26*0~
L31 DTP*292*RD8*20170101-20171231~
L32 EB*B**30*MA**26*0~
L33 HSD***DA**30*0~
L34 HSD***DA**31*60~
L35 HSD*****26*1~
L36 DTP*435*RD8*20170101-20171231~
...
```
(added line number at beginning of each line so we can reference them)

The issue is with the schema, where inside `EB` seg group, we have, in this particular order,
the following child segments: `EB`, `DTP`, `LS`, `HSD`. You can see the `DTP` seg is declared
in front of `HSD`, but in the input it appears after. Now both `DTP` and `HSD` seg decl has
`min = 0`. When edi reader sees the 3 `HSD`, it thinks it's okay, since `DTP` seg is optional.
Then edi reader sees the `DTP`. Unfortunately this time it can't find a proper placement match
for it, thus it considers the `EB` seg group is done. Cascadingly, it considers the wrapping
`transaction_set_id` seg group as well as the `GS` and `ISA` are all done. Thus it moves on.
Then it sees `SE` and `IEA` both are optional, so seems like to it everything is done. EDI
reader now rewinds the seg processing stacking all the way to the #root. At this moment, essentially
we have a dangling raw segment `DTP` unprocessed and unaccounted for. EDI reader should've
gracefully failed, but it didn't. Due a bug, it will try to mark the current instance of #root
seg decl done, and move onto the next instance of #root decl. But again, `DTP` is still not matched
in the next instance of #root (the first segment in this schema should always be `ISA`), it will
move on to the next instance of #root, yet again and again and again - infinite dead-loop.

The fix is:

If we can't match a raw seg name to the current decl stack, and if the current decl stack is
nothing but the virtual #root decl, then we have a dangling unmated seg and return an failure
immediately.
@codecov
Copy link

codecov bot commented Mar 28, 2021

Codecov Report

Merging #143 (02be372) into master (675471c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #143   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           43        43           
  Lines         1977      1983    +6     
=========================================
+ Hits          1977      1983    +6     
Impacted Files Coverage Δ
extensions/omniv21/fileformat/edi/reader.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 675471c...02be372. Read the comment docs.

@jf-tech jf-tech merged commit 9232c62 into master Mar 28, 2021
@jf-tech jf-tech deleted the issue_141 branch March 28, 2021 21:36
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

Successfully merging this pull request may close these issues.

Parser only loops 10 times
1 participant