jf-tech / omniparser

omniparser: a native Golang ETL streaming parser and transform library for CSV, JSON, XML, EDI, text, etc.
MIT License
931 stars 68 forks source link

Multi-lined envelope reading corruption caused by direct reference into bufio.Reader's internal buffer rollover. #214

Closed jf-tech closed 1 year ago

jf-tech commented 1 year ago

BUG: https://github.com/jf-tech/omniparser/issues/213

If we're dealing with multi-lined envelope (either by rows or by header/footer), readLine() will be called several times, thus whatever ios.ByteReadLine, which uses bufio.Reader underneath, returns in a previous call may be potentially be invalidated due to bufio.Reader's internal buf rollover. If we read the previous line directly, it would cause corruption.

To fix the problem the easiest solution would be simply copying the return []byte from ios.ByteReadLine every single time. But for files with single-line envelope, which are the vast majority cases, this copy becomes unnecessary and burdensome on gc. So the trick is to has a flag on reader.linesBuf's last element to tell if it contains a reference into the bufio.Reader's internal buffer, or it's a copy. Every time before we call bufio.Reader read, we check reader.liensBuf's last element flag, if it is not a copy, then we will turn it into a copy.

This way, we optimize for the vast majority cases without needing allocations, and avoid any potential corruptions in the multi-lined envelope cases.

@paulstadler

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (37370fa) 100.00% compared to head (5b7e14d) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #214 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 53 53 Lines 3021 3027 +6 ========================================= + Hits 3021 3027 +6 ``` | [Impacted Files](https://app.codecov.io/gh/jf-tech/omniparser/pull/214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JF+Technology) | Coverage Δ | | |---|---|---| | [.../omniv21/fileformat/flatfile/fixedlength/reader.go](https://app.codecov.io/gh/jf-tech/omniparser/pull/214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JF+Technology#diff-ZXh0ZW5zaW9ucy9vbW5pdjIxL2ZpbGVmb3JtYXQvZmxhdGZpbGUvZml4ZWRsZW5ndGgvcmVhZGVyLmdv) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.