search

mustang-im / mustang

Mustang - New full-featured desktop email, chat and video conference client
https://mustang.im
Other
8 stars 1 forks source link

EWS: Sanitize all input #71

Closed benbucksch closed 3 months ago

benbucksch commented 3 months ago

All input read from the network must be sanitized, normally using the sanitize class. See how IMAPEMail.fromFlow() and EMail.parseMIME() do that.

I've added a new angle to the sanitize functions: The last parameter is a fallback. If the original value does not exist, or if it fails validation, the sanitize function will return that fallback value. If the fallback value is not passed, it will throw instead (which was the behavior). This allows you to write code that is error-resilient and relatively compact.

All basic and most common data types, like string, label, integer, date, emailAddress, url, hostname, string enum etc. have corresponding validation functions. Please read through the class, which functions it offers.

benbucksch commented 3 months ago

This applies to EWSEMail, EWSPerson, EWSEvent and all other EWS* objects that read from the network or from disk.

NeilRashbrook commented 3 months ago

Ugh, it hurts my soul to run strings through sanitize.string... and not just because it's misspelt in my locale...

benbucksch commented 3 months ago

:rofl:

benbucksch commented 3 months ago

You are welcome to import it as sanitise :rofl:

benbucksch commented 3 months ago

Re string: Whenever possible, try to use more specific validation functions, e.g. date, emailAddress, label, nonemptystring etc.

NeilRashbrook commented 3 months ago

It's fortunate that the user and domain checks in sanitize.emailAddress are useless because they wouldn't throw if there was a fallback anyway.

It's even more fortunate that you asked me to sanitise everything because I found a logic error (which I already suspected to exist but I spotted it in the code while I was adding the sanitisation).

benbucksch commented 3 months ago

@NeilRashbrook Is this complete? If so, could you please close it?

NeilRashbrook commented 3 months ago

Yes, this was merged as part of PR #89.