14

Why does System.IOUtils.TPath.HasValidPathChars accept'?' as a valid char in a path? I set the second parameter (UseWildcards) to false. So, according to the documentation the '?' should be rejected. Still, the function returns True for 'c:\test\test?\'.

UseWildcards = Specifies whether the mask characters are treated as valid path characters (e.g. asterisk or question mark).

Is the behavior of this function only partially correct? Could the function have returned a better result?

Boann
  • 48,794
  • 16
  • 117
  • 146
Gabriel
  • 20,797
  • 27
  • 159
  • 293
  • 2
    The `?` should not be rejected because paths CAN contain question marks on Windows. Example: https://superuser.com/q/1069055 – Günther the Beautiful Jul 27 '17 at 09:31
  • That superuser post also states: "You can't use these kinds of paths to access files or directories in userspace. Only certain low-level system components are designed to work with Object Manager paths." So from this viewpoint `?` is an invalid path character. – gabr Jul 27 '17 at 09:34
  • @GünthertheBeautiful - The article says that \?? is a valid path, so HasValidPathChars should check for '??' and return true IF the correct pattern ( \??) is found and false when there is a single '?' – Gabriel Jul 27 '17 at 09:36
  • @GünthertheBeautiful `HasValidPathChars` recognises that with its use of `GetPosAfterExtendedPrefix`, but the rest of the function is hopelessly broken. – David Heffernan Jul 27 '17 at 09:51

1 Answers1

18

TPath.HasValidPathChars is completely broken. This is its implementation:

class function TPath.HasValidPathChars(const Path: string;
  const UseWildcards: Boolean): Boolean;
var
  PPath: PChar;
  PathLen: Integer;
  Ch: Char;
  I: Integer;
begin
  // Result will become True if an invalid path char is found
{$IFDEF MSWINDOWS}
  I := GetPosAfterExtendedPrefix(Path) - 1;
{$ENDIF MSWINDOWS}
{$IFDEF POSIX}
  I := 0;
{$ENDIF POSIX}

  PPath := PChar(Path);
  PathLen := Length(Path);
  Result := False;

  while (not Result) and (i < PathLen) do
  begin
    Ch := PPath[i];
    if not IsValidPathChar(Ch) then
      if UseWildcards then
        if not IsPathWildcardChar(Ch) then
          Result := True
        else
          Inc(i)
      else
        Result := True
    else
      Inc(i);
  end;

  Result := not Result;
end;

The crucial point is the call to IsValidPathChar. Let's look at what that does.

class function TPath.IsValidPathChar(const AChar: Char): Boolean;
begin
  Result := not IsCharInOrderedArray(AChar, FInvalidPathChars);
end;

Now, FInvalidPathChars is defined to be:

FInvalidPathChars := TCharArray.Create(
  #0, #1, #2, #3, #4, #5, #6, #7, #8, #9, #10, #11, #12,
  #13, #14, #15, #16, #17, #18, #19, #20, #21, #22, #23, #24,
  #25, #26, #27, #28, #29, #30, #31,
  '"', '<', '>', '|');            // DO NOT LOCALIZE;

That is, all ordinals less than 32, and ", <, > and |.

We also need to understand what IsPathWildcardChar does.

class function TPath.IsPathWildcardChar(const AChar: Char): Boolean;
begin
  Result := IsCharInOrderedArray(AChar, FPathWildcardChars);
end;

Where FPathWildcardChars is:

FPathWildcardChars := TCharArray.Create('*', '/', ':', '?', '\'); // DO NOT LOCALIZE;

Now, back to TPath.HasValidPathChars. Let's consider this if statement:

if not IsValidPathChar(Ch) then

The condition not IsValidPathChar(Ch) evaluates to True when IsValidPathChar(Ch) is False. Which happens if Ch is in FInvalidPathChars. That is if Ch has ordinal less than 32, or is one of ", <, > and |.

Your test string is 'C:\test\test?\' and in fact none of these characters are in FInvalidPathChars. Which means that the condition in the if not IsValidPathChar(Ch) then statement always evaluates False. So even though your string contains a wildcard, it can never reach the subsequent test:

if UseWildcards then

It is easy to conclude that HasValidPathChars returns the same value irrespective of the value of the input parameter UseWildcards. And if you have any doubt about the analysis, this program should dispel it:

{$APPTYPE CONSOLE}

uses
  System.SysUtils,
  System.IOUtils;

procedure Main;
var
  Ch: Char;
begin
  for Ch := low(Ch) to high(Ch) do
    if TPath.HasValidPathChars(Ch, False)<>TPath.HasValidPathChars(Ch, True) then
      Writeln('different at #' + IntToStr(ord(Ch)));
  Writeln('finished');
end;

begin
  Main;
  Readln;
end.

This looks like yet another function in this dreaded IOUtils unit that has been improperly implemented and not tested.

I have submitted a bug report: RSP-18696.

Based on having stumbled upon many such problems with IOUtils, my experience is that the unit is not to be trusted. I would not use it. Find an alternative way to solve your problem.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • "It is easy to conclude that HasValidPathChars returns the same value irrespective of the value of the input parameter UseWildcards" - I just wanted to say exactly that!!! – Gabriel Jul 27 '17 at 09:53
  • Thanks David. "my experience is that the unit is not to be trusted" - I had my own 'I/O utils' library but I replaced it with Embarcadero's when I purchased Delphi XE because... well... it was built by Embarcadero... so it must be better than mine... right? I am not top Delphi programmer so I expect that the worst programmer at Embarcadero to be at least 3 time better than me (he should be a TRUE Delphi professional, right?) !!!!!!!!!!!!!! How they can let out such huge mistakes out? Probably this is why the IDE crashes so much: they used IOutils in their IDE code :) – Gabriel Jul 27 '17 at 10:04
  • @Edwin At least it is open. The defect that was hurting me yesterday was submitted 18 months ago and is still "reported". – David Heffernan Jun 04 '19 at 06:43