Search code examples
delphipathdirectorydelphi-xe7

Why does TPath.HasValidPathChars accept '?' as a valid char in a path?


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?


Solution

  • 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.