byte-physics / igortest

Igor Pro Universal Testing Framework
https://docs.byte-physics.de/igor-unit-testing-framework/
BSD 3-Clause "New" or "Revised" License
7 stars 2 forks source link

Fix: Replace if(strlen(str)) with UTF_Utils#IsEmpty #407

Closed Garados007 closed 1 year ago

Garados007 commented 1 year ago

Strings from function arguments can be null and should be untrusted. Using if(strlen(str)) to the check if its empty is therefore insecure and prone to errors. These calls have been replaced with the better UTF_Utils#IsEmpty.

Not all if(strlen(str)) are insecure and therefore only the mentioned case is updated. If the string originate from a wave or an SVAR, which cannot be null, the code if(strlen(str)) is kept and not replaced.

Close #303

Garados007 commented 1 year ago

@t-b @MichaelHuth Ready for review.

t-b commented 1 year ago

Not all if(strlen(str)) are insecure and therefore only the mentioned case is updated. If the string originate from a wave or an SVAR, which cannot be null, the code if(strlen(str)) is kept and not replaced.

I would actually prefer if all strlen() on plain strings which are used for checking empty/filled are replaced with IsEmpty. We could even move to IsEmpty taking a copy so that this can be used with structure elements and SVARs as well. We have done that in MIES already, see AllenInstitute/MIES@9857c4bf (Changed IsEmpty to not use string by reference, 2021-09-14)

Garados007 commented 1 year ago

Okay, I will update IsEmpty and it's references.

Garados007 commented 1 year ago

@MichaelHuth

The behavior changes need some documentation.

What kind of documentation do you think of? The function documentation has been updated, the commit message mention it and IsEmpty is nowhere used or mentioned in our sphinx documentation. Should I add something to the changelog?

IsEmpty was an internal function all the time and is hidden with ///@cond HIDDEN_SYMBOL. I don't think there are external user.

Garados007 commented 1 year ago

Removed IsNullOrEmpty as discussed yesterday.