lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Path validation in boost and std::fs


From: Greg Chicares
Subject: Re: [lmi] Path validation in boost and std::fs
Date: Sat, 17 Mar 2018 14:43:32 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 2018-03-16 18:49, Vadim Zeitlin wrote:
> 
>  I'd like to return to this topic to discuss how to handle an important
> difference between Boost.Filesystem and std::filesystem libraries. The
> former one has the concept of "checkers", which are used whenever paths are
> constructed to verify their validity, while the latter one seems to take a
> much more relaxed, Unix-like, approach and doesn't impose any restrictions
> at all on the path components.

Do you know why they removed it? Using 'name_check' as a search term,
I don't find any discussion on lists.boost.org . Just curious.

IIRC, for lmi the name_check policy was only a nuisance that needed
to be worked around by setting
  fs::path::default_name_check(fs::native);
before any other filesystem function could be called (e.g., from the
ctor of a global object). If we failed to do that, then customary
msw-native paths would be rejected. Maybe that was boost's reason
for removing the feature.

>  This affects a lot of operations, even when it might not be expected: for
> example, the unit test checking that fs::change_extension("..", "..")
> throws, which passes with Boost 1.33 because the "..." sequence fails a
> validity check, starts failing after rewriting it as mostly equivalent
> fs::path("..").replace_extension("..") as this just produces "...." path
> without any exceptions being thrown. Other unit tests fail because
> validate_path() now doesn't check for any invalid characters in the path,
> while the tests expect it to do it for characters such as '<', '|' and '>'.

Sometimes I add unit tests to validate the exceptions that may be
thrown, just to make sure they actually are thrown. For example,
validate_filepath() throws if any of its enumerated preconditions
fails. I certainly wanted to unit-test the non-empty and existence
checks, to be sure they work as intended, and I had to trap nuisance
exceptions thrown by boost, so I tested those too--just in order to
cover every exceptional path. Remove the exception, and there's no
need to test whether it's trapped.

>  Moreover, I'm not really sure how much value do these checks represent.
> Any attempt to actually use a path with invalid characters in it would fail
> anyhow under systems imposing such restrictions (i.e. MSW), so what do we
> gain from checking for them preventively?

It seems good to prevent users from making mistakes like:
  File | Save as
    `XYZ' Corp.[#2 of 3] (managers and/or professionals?) <New York *only*>.
  OK
I guess that was boost's original rationale for the name_check
mechanism. If paths are checked preemptively, then in principle we
can trap insane names up front, and report them as elaborately
as we like...e.g., instead of letting a downstream library issue
a cryptic diagnostic such as "xml: parse error", or, worse, a
screenful of XSL-FO diagnostics when a name is acceptable to msw
but not to java.

>  There is also a compromise solution of only performing these checks in
> validate_path() itself. This would then allow to do such checks when we
> really need them, while still allowing us to use just fs::path directly
> elsewhere.

Yes, that's perfect. I think the platform-specific rules boost
used are simple enough to clone.

We use validate_path() rarely--AFAICS, only for directories and
files controlled by configurable_settings or global_settings,
whose validity is crucial.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]