Reject FHIR resources with control chars #2605

[prb112]

Originally I started off at the fhir-server module, and wrote a ReadInterceptor with a ReadableByteChannel which scanned for 1,2,3,4 byte sequences with forbidden bytes. It's a bit more complicated as they can be encoded as entity values or escaped unicode and not in the natural bytes.

I ended up taking those tests and pushing the tests down into fhir-model along with a test case from @lmsurpre and hardened ValidationSupport to check for Unicode characters which are forbidden.

The key changes are in ValidationSupport.

Signed-off-by: Paul Bastide pbastide@us.ibm.com

[punktilious]

I think we need to compare performance.

[prb112]

I've updated the issue with my feedback. #2605 (comment)
Net, after testing my original SET contains is the most efficient by about 30% and slower than the Original implementation by about 30%.

I have some optimizations that I've made to StringFormat usage for the error message assembly, that speeds processing up. The changes are captured in the comparision of the Original,Set,Byte/Integer check.

All my tests were are the latest adoptopenjdk 11

[prb112]

I think we need to compare performance.

included as a reply in the original issue and as noted above.

[lmsurpre]

even at 30% and slower, I don't think this method will even register against the total time to parse a resource

[lmsurpre]

I approved, but I don't think it addresses the "add a ModelConfig option to make this a warning instead of an error" part of the issue yet: #2605 (comment)

Let me know if I just missed it, or if you're thinking to do a followup PR for that part.

[prb112]

I approved, but I don't think it addresses the "add a ModelConfig option to make this a warning instead of an error" part of the issue yet: #2605 (comment)

Let me know if I just missed it, or if you're thinking to do a followup PR for that part.

I was layering this as two separate changes to discuss; I plan to add the ModelConfig change with this PR.

[prb112]

Implemented, waiting on ci/cd

[punktilious]

LGTM