Recent

Author Topic: ExpandFileName seems wrong with multiple path separators next to each other.  (Read 3232 times)

comingnine

  • New Member
  • *
  • Posts: 25
In the example, three paths are expanded with ExpandFileName. The result when there are multiple path separators seems to be wrong. 

Code: Pascal  [Select][+][-]
  1. program Project1;
  2.  
  3. {$IFNDEF FPC}{$APPTYPE CONSOLE}{$ENDIF}
  4.  
  5. uses
  6.   SysUtils;
  7.  
  8. begin
  9.   Writeln(ExpandFileName('/var/log//../'));
  10.   Writeln(ExpandFileName('/var/log/../'));
  11.   Writeln(ExpandFileName('/var/log/..'));
  12. end.  

output if compiled with fpc for linux
Code: Pascal  [Select][+][-]
  1. /var/log/
  2. /var/
  3. /var

output if compiled with fpc for windows
Code: Pascal  [Select][+][-]
  1. D:\var\log\
  2. D:\var\
  3. D:\var

output if compiled with kylix
Code: Pascal  [Select][+][-]
  1. /var/
  2. /var/
  3. /var

output if compiled with delphi
Code: Pascal  [Select][+][-]
  1. D:\var\
  2. D:\var\
  3. D:\var


Code: Pascal  [Select][+][-]
  1. $ realpath /var/log//../
  2. /var
  3. $ realpath /var/log/../
  4. /var
  5. $ realpath /var/log/..
  6. /var

eny

  • Hero Member
  • *****
  • Posts: 1609
output if compiled with fpc for windows
Code: Pascal  [Select][+][-]
  1. D:\var\log\
That one is definitely wrong because Windows itself (Win10) recognizes it as a route to D:\var.

Although probably an exception would be better because it does not look like a valid path name.
Even though apparently OS's seem to try to do their best to make something useful out of it.
All posts based on: Win10 (Win64); Lazarus 2.0.10 'stable' (x64) unless specified otherwise...

comingnine

  • New Member
  • *
  • Posts: 25
output if compiled with fpc for windows
Code: Pascal  [Select][+][-]
  1. D:\var\log\
That one is definitely wrong because Windows itself (Win10) recognizes it as a route to D:\var.

Although probably an exception would be better because it does not look like a valid path name.
Even though apparently OS's seem to try to do their best to make something useful out of it.

I should be more specific. What I mean is when there are multiple path separators, FPC's ExpandFileName fails to handle "..".

lucamar

  • Hero Member
  • *****
  • Posts: 3084
I should be more specific. What I mean is when there are multiple path separators, FPC's ExpandFileName fails to handle "..".

Well, that depends on the point of view: it *does* go one level up ... which just happen to be empty  :)

Now seriously, what it does wrong (if for nothing else than Delphi compatiility) is precisely that: not realizing that the level-up results in an empty name. Either that, or not normalicing the path---converting double separators to single pnes---before processing it.

In either case, it shouldn't be too hard to correct it.
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.8/FPC 3.0.4 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

Bart

  • Hero Member
  • *****
  • Posts: 3952
    • Bart en Mariska's Webstek
ExpandFilenameUTF8 form Lazutils seesm to work correctly.
Code: [Select]
c:\devel\fpc\\..\ -> c:\devel\
c:\devel\fpc\\.. -> c:\devel
c:\devel\fpc\..\ -> c:\devel\
c:\devel\fpc\.. -> c:\devel

Bart

ASerge

  • Hero Member
  • *****
  • Posts: 1682
ExpandFilenameUTF8 form Lazutils seesm to work correctly.
From LazFileUtils of course.
By the way, removing the double slash (not at the beginning of the name) is not documented in Window (unlike *nix), but is logical. This allows to correct incorrect path concatenation, given that such a character can not be in the file name and has no consequences

lucamar

  • Hero Member
  • *****
  • Posts: 3084
Yet another fix, if for some reason you don't want to pull a Lazarus unit, is to make sure to never pass a doubled path delimiter to ExpandFileName. Like, for example, in this (thoroughly untested code):

Code: Pascal  [Select][+][-]
  1. program Project1;
  2.  
  3. {$IFNDEF FPC}{$APPTYPE CONSOLE}{$ENDIF}
  4.  
  5. uses
  6.   SysUtils, StrUtils;
  7.  
  8. function FixDoubleDelim(APath: String): String;
  9. {Note: UNC checks in this code are *rubbish*, just an example. Use a more robust solution if you need it!}
  10. var
  11.   IsUNC: Boolean;
  12.   DoubleDelim:  String;
  13. begin
  14.   DoubleDelim := DirectorySeparator + DirectorySeparator;
  15. {$IFNDEF WINDOWS}
  16.   IsUNC := False;
  17. {$ELSE}
  18.   IsUNC := Pos(DoubleDelim, APath) = 1;
  19. {$ENDIF}
  20.   {Using a "while" here is probably overkill, but ... Murphy!}
  21.   while Pos(DoubleDelim, APath) > 0 do
  22.     AnsiReplaceString(APath, DoubleDelim, DirectorySeparator);
  23.   if IsUNC then
  24.     Result := '/' + APath
  25.   else
  26.     Result := APath;
  27. end;
  28.  
  29. begin
  30.   Writeln(ExpandFileName(FixDoubleDelim('/var/log//../')));
  31.   Writeln(ExpandFileName(FixDoubleDelim('/var/log/../')));
  32.   Writeln(ExpandFileName(FixDoubleDelim('/var/log/..')));
  33. end.

Care must be taken, of course, to make sure that the target OS doesn't admit doubled delimiters! :D
« Last Edit: August 21, 2018, 02:32:25 am by lucamar »
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.8/FPC 3.0.4 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

Bart

  • Hero Member
  • *****
  • Posts: 3952
    • Bart en Mariska's Webstek
Fpc or Lazarus has some IsUNC() function... (probably for Lazarus in lazfileutils.pp or winlazfileutils.inc in components/lazutils folder)
The while loop on StringReplace is in my experience not redundant.
IIRC then if you have '///', after one StringReplace you end up with '//'.
(Cannot test right now, no fpc available to me ATM).

Bart

Bart

  • Hero Member
  • *****
  • Posts: 3952
    • Bart en Mariska's Webstek
Can somebody also please test what Delphi does.
If it differs from fpc, a bugreport whould be filed.

Bart

marcov

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 8800
  • FPC developer.
Best is to file a bug with FPC. The example program of comingnine is already fine for that.  Tomas has been working on this problem (fexpand/expandfilename) since FPC 1.x times, so getting him on board is important. It's quite complex because UNC and Amiga/Novel volume notations.

While there is only expandfilename routine, it is parameterized with ifdefs, there is a small chance that the outcome per OS could be different. So add test results for at least windows and *nix to the report

ASerge

  • Hero Member
  • *****
  • Posts: 1682
Can somebody also please test what Delphi does.
What test? The result in Delphi is already at the beginning of the topic. It uses GetFullPathName with the same result as ExpandFilenameUTF8.

marcov

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 8800
  • FPC developer.
What test? The result in Delphi is already at the beginning of the topic. It uses GetFullPathName with the same result as ExpandFilenameUTF8.

getfullpathnamew is different. It requires path to actually exist afaik, and expandfilename does not?

Bart

  • Hero Member
  • *****
  • Posts: 3952
    • Bart en Mariska's Webstek
Best is to file a bug with FPC.

Done: Issue #34166.

Bart

ASerge

  • Hero Member
  • *****
  • Posts: 1682
getfullpathnamew is different. It requires path to actually exist afaik, and expandfilename does not?
From docs link:
Quote
This function does not verify that the resulting path and file name are valid, or that they see an existing file on the associated volume

 

TinyPortal © 2005-2018