Re: [lmi] Validating paths with std::filesystem

[Vadim Zeitlin]

On Thu, 8 Oct 2020 17:49:50 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> On 2020-10-08 13:22, Vadim Zeitlin wrote:
[...]
GC> >  Do we need to validate paths at all when using std::filesystem?
GC> 
GC> TL;DR: your option 0 below.

 Thanks, this answers my question, so the rest is purely informational.

GC> To reproduce:
GC> 
GC> $vim -p /opt/lmi/data/configurable_settings.xml
GC> Cause the <print_directory> element to be as follows:
GC>   
<print_directory>/opt/lmi/src/lmi/Z:/etc/opt/lmi/default.ill</print_directory>

 OK, this was the only way to reproduce it that we've found too, but it
seemed implausible to me that you'd actually have such a path in your
configurable_settings.xml. But it turns out that the classic rule about the
only alternative remaining after discarding all the impossible ones being
true even if it seems implausible is still correct. Thanks for confirming
this!

GC> In this case, I looked back at
GC>   $git show 090d19a80e6a
GC> and remembered the rationale. We made the "print" directory configurable.
GC> Then we asked ourselves "what if it's configured invalidly?", and decided
GC> to modify it forcibly, in class configurable_settings's ctor, displaying
GC> a messagebox announcing the problem and explaining the resolution. That
GC> seemed perfect at the time.

 FWIW it doesn't seem perfect to me, I see no reason to perform this check
on startup. We don't even know if the print directory is going to be used,
so why bother checking it? I'd wait until we actually do need it and then
try to find a solution interactively, i.e. allowing the user to select the
directory to use it or cancel the operation which needs this directory.

GC> However, wise users don't violate pathname rules. And if unwise users
GC> try to save an lmi input file as "¯\_(ツ)_/¯.cns" or "LPT1.ill", I'd
GC> ask them to save a spreadsheet as "LPT1.xlsx" and see what happens.

 FWIW I already know what would happen, but here is the result of the
experiment I've just done: on pressing "Save" in the file dialog after
entering "LPT1.xsls" in it you get the following message box

        [Window Title]
        Save As

        [Content]
        c:\lpt1.xlsx
        This file name is reserved for use by Windows.
        Choose another name and try again.

        [OK]

I.e. MSW file dialogs always perform validation. And it's, actually, the
only argument to do it in lmi that I see: if you get an error when entering
LPT1.ill interactively, it might make sense to also give the same, or
similar, error when using it in some other way. But OTOH it could be argued
that interactive use is a special use case.

GC> 
GC> >  So, as usual, there are several choices:
GC> > 
GC> > 0. Don't implement any validation at all.
GC> 
GC> Yes, that's my preference, too.
GC> 
GC> > 1. Implement minimal validation exactly reproducing the current behaviour.
GC> > 2. Implement [as] full validation [as possible].
GC> > 
GC> >  I think (0) should actually be fine, but if not, then I'd rather do (2)
GC> > than (1).
GC> 
GC> There's at least one other option:
GC> 
GC>   3. Do what they should have done, and transform their input into
GC> something sensible.

 FWIW I strongly dislike this. I think a program should never
unconditionally transform user input into something else unless it's
absolutely certain about what the user meant. And here we just have no way
of knowing what to replace lpt.ill with, i.e. anything that we could use
would be quite surprising to the user. Returning an error seems to be a
much better choice.

 Regards,
VZ

pgpw8cq9xePrZ.pgp
Description: PGP signature