Fixes Exceptions and permanent Hang when parsing bad input, Adds bad input tests#197
Fixes Exceptions and permanent Hang when parsing bad input, Adds bad input tests#197milkshakeuk merged 10 commits intonHapiNET:masterfrom kulgg:master
Conversation
This comment has been minimized.
This comment has been minimized.
milkshakeuk
left a comment
There was a problem hiding this comment.
Looks mostly fine, just a few comments, left me know what you think.
This comment has been minimized.
This comment has been minimized.
|
Fixes #198 |
This comment has been minimized.
This comment has been minimized.
|
@JlKmn did you see some of the other comments? |
|
@milkshakeuk I think i answered to all of them, what am I missing? |
|
@JlKmn |
|
@milkshakeuk Very strange they didn't show for me on brave and edge. I can hover over the link and see some of the comment but can't click on it. They also don't show for me in Files changed with comments? |
|
Can you comment them again |
This comment has been minimized.
This comment has been minimized.
|
@JlKmn |
…handled exception bugs and the permanent Hang
|
@milkshakeuk should be fine now right? |
Unit Test Results 5 files 56 suites 8s ⏱️ Results for commit fbde865. |
|
@JlKmn Yes thanks! |
|
@JlKmn I have rebased my branch from master so it now includes your changes however now some of my tests are failing because of changes I have made to |
|
@milkshakeuk ok how do we go about that |
|
@JlKmn Once my PR is ready ill push up a branch which you could fork and have a look at? |
|
@milkshakeuk ok |
|
@JlKmn ok here is my branch. |
|
@milkshakeuk ok i will look at it soon |
|
@milkshakeuk I looked at it and the changes in exceptions for the tests happen because you rewrote the Parse function in ParserBase.cs that gets called for every BadInput test. One fix would be to extend the one test to accept EncodingNotSupported because it is a HL7 child exception. Maybe move the others to the Gracefully test? |
|
@JlKmn ok I'll look to make those changes. |
|
@milkshakeuk Yes |
|
@JlKmn ok the PR has been merged. |
|
@milkshakeuk I will do a Fuzzing run on the new Parser tonight, we'll see tomorrow |
Fixes #196 and #198