Skip to content

Commit 9232c62

Browse files
authored
Fix ISSUE-141: edi reader stuck in a dead-loop with unrecognized raw segment at the end of the input. (#143)
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.
1 parent 675471c commit 9232c62

File tree

4 files changed

+94
-25
lines changed

4 files changed

+94
-25
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"Records": [
3+
"{'IEA':{},'ISA':{}}",
4+
"{'IEA':{},'ISA':{}}"
5+
],
6+
"FinalErr": "EOF"
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"Records": [
3+
"{}"
4+
],
5+
"FinalErr": "input 'test' at segment no.2 (char[13,13]): segment 'UNKNOWN' is either not declared in schema or appears in an invalid order"
6+
}

extensions/omniv21/fileformat/edi/reader.go

+12
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,20 @@ func (r *ediReader) Read() (*idr.Node, error) {
231231
if err != nil {
232232
return nil, err
233233
}
234+
if len(r.stack) <= 1 {
235+
// we have a raw segment waiting for processing but currently
236+
// the decl stack is effectively empty (with only the artificial
237+
// #root decl on it. We can get into this situation two ways:
238+
// 1.
239+
}
234240
cur := r.stackTop()
235241
if !cur.segDecl.matchSegName(rawSeg.Name) {
242+
if len(r.stack) <= 1 {
243+
return nil, ErrInvalidEDI(r.fmtErrStr2(
244+
r.r.SegCount(), r.r.RuneEnd(), r.r.RuneEnd(),
245+
"segment '%s' is either not declared in schema or appears in an invalid order",
246+
rawSeg.Name))
247+
}
236248
err = r.segNext()
237249
if err != nil {
238250
return nil, err

extensions/omniv21/fileformat/edi/reader_test.go

+69-25
Original file line numberDiff line numberDiff line change
@@ -586,11 +586,11 @@ func TestSegDoneSegNext(t *testing.T) {
586586

587587
func TestRead(t *testing.T) {
588588
for _, test := range []struct {
589-
name string
590-
input string
591-
declJSON string
592-
xpath string
593-
err string
589+
name string
590+
input string
591+
declJSON string
592+
xpath string
593+
readerCreationErr string
594594
}{
595595
{
596596
name: "invalid target xpath, failure",
@@ -603,8 +603,8 @@ func TestRead(t *testing.T) {
603603
{ "name": "ISA", "min": 0 }
604604
]
605605
}`,
606-
xpath: "[",
607-
err: `invalid target xpath '[', err: expression must evaluate to a node-set`,
606+
xpath: "[",
607+
readerCreationErr: `invalid target xpath '[', err: expression must evaluate to a node-set`,
608608
},
609609
{
610610
name: "empty input, success",
@@ -617,8 +617,8 @@ func TestRead(t *testing.T) {
617617
{ "name": "ISA", "min": 0 }
618618
]
619619
}`,
620-
xpath: "",
621-
err: "",
620+
xpath: "",
621+
readerCreationErr: "",
622622
},
623623
{
624624
name: "single seg decl, multiple seg instances, success",
@@ -640,8 +640,8 @@ func TestRead(t *testing.T) {
640640
}
641641
]
642642
}`,
643-
xpath: "",
644-
err: "",
643+
xpath: "",
644+
readerCreationErr: "",
645645
},
646646
{
647647
name: "2 seg decls, success",
@@ -669,8 +669,8 @@ func TestRead(t *testing.T) {
669669
}
670670
]
671671
}`,
672-
xpath: "",
673-
err: "",
672+
xpath: "",
673+
readerCreationErr: "",
674674
},
675675
{
676676
name: "2 seg groups, filtered target, success",
@@ -711,8 +711,8 @@ func TestRead(t *testing.T) {
711711
}
712712
]
713713
}`,
714-
xpath: ".[e1 != '6']",
715-
err: "",
714+
xpath: ".[e1 != '6']",
715+
readerCreationErr: "",
716716
},
717717
{
718718
name: "seg min not satisfied before EOF, failure",
@@ -740,8 +740,8 @@ func TestRead(t *testing.T) {
740740
}
741741
]
742742
}`,
743-
xpath: "",
744-
err: "",
743+
xpath: "",
744+
readerCreationErr: "",
745745
},
746746
{
747747
name: "missing raw seg name, failure",
@@ -762,8 +762,8 @@ func TestRead(t *testing.T) {
762762
}
763763
]
764764
}`,
765-
xpath: "",
766-
err: "",
765+
xpath: "",
766+
readerCreationErr: "",
767767
},
768768
{
769769
name: "raw seg processing wrong, failure",
@@ -784,8 +784,8 @@ func TestRead(t *testing.T) {
784784
}
785785
]
786786
}`,
787-
xpath: "",
788-
err: "",
787+
xpath: "",
788+
readerCreationErr: "",
789789
},
790790
{
791791
name: "seg min not satisfied before next seg appearance, failure",
@@ -810,18 +810,62 @@ func TestRead(t *testing.T) {
810810
}
811811
]
812812
}`,
813-
xpath: "",
814-
err: "",
813+
xpath: "",
814+
readerCreationErr: "",
815+
},
816+
{
817+
name: "unprocessed raw segment, failure",
818+
input: "ISA\nUNKNOWN\n",
819+
declJSON: `
820+
{
821+
"segment_delimiter": "\n",
822+
"element_delimiter": "*",
823+
"segment_declarations": [
824+
{
825+
"name": "ISA",
826+
"is_target": true,
827+
"min": 0
828+
}
829+
]
830+
}`,
831+
xpath: "",
832+
readerCreationErr: "",
833+
},
834+
{
835+
name: "multiple root level segments, success",
836+
input: "ISA\nIEA\nISA\nIEA\n",
837+
declJSON: `
838+
{
839+
"segment_delimiter": "\n",
840+
"element_delimiter": "*",
841+
"segment_declarations": [
842+
{
843+
"name": "group1",
844+
"type": "segment_group",
845+
"is_target": true,
846+
"child_segments": [
847+
{
848+
"name": "ISA"
849+
},
850+
{
851+
"name": "IEA"
852+
}
853+
]
854+
}
855+
]
856+
}`,
857+
xpath: "",
858+
readerCreationErr: "",
815859
},
816860
} {
817861
t.Run(test.name, func(t *testing.T) {
818862
var decl FileDecl
819863
err := json.Unmarshal([]byte(test.declJSON), &decl)
820864
assert.NoError(t, err)
821865
reader, err := NewReader("test", strings.NewReader(test.input), &decl, test.xpath)
822-
if test.err != "" {
866+
if test.readerCreationErr != "" {
823867
assert.Error(t, err)
824-
assert.Equal(t, test.err, err.Error())
868+
assert.Equal(t, test.readerCreationErr, err.Error())
825869
assert.Nil(t, reader)
826870
return
827871
}

0 commit comments

Comments
 (0)