View Issue Details

IDProjectCategoryView StatusLast Update
0034166FPCRTLpublic2018-09-29 19:11
ReporterBart BroersmaAssigned ToTomas Hajny 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindowsOS Version10
Product Version3.1.1Product Buildr39352 
Target VersionFixed in Version3.3.1 
Summary0034166: ExpandFilename does not handle double DirectorySeparator followed by .. in a path
DescriptionExpandFilename returns wrong result if path contains a double DirectorySeparator followed by ..
Steps To Reproduceprogram expfn;

{$apptype console} //delphi 7 does not parse this if it is inside ifdef windows

{$ifdef fpc}
{$mode objfpc}{$h+}
{$endif}

uses
  sysutils;

begin
  writeln('c:\devel\fpc\\..\ -> ',expandfilename('c:\devel\fpc\\..\'));
  writeln('c:\devel\fpc\\.. -> ',expandfilename('c:\devel\fpc\\..'));
  writeln('c:\devel\fpc\..\ -> ',expandfilename('c:\devel\fpc\..\'));
  writeln('c:\devel\fpc\.. -> ',expandfilename('c:\devel\fpc\..'));
end.

C:\Users\Bart\LazarusProjecten\bugs\Console\expandfilename>fpc expfn.lpr
Free Pascal Compiler version 3.1.1 [2018/07/18] for i386
...
18 lines compiled, 0.2 sec, 69760 bytes code, 4196 bytes data

C:\Users\Bart\LazarusProjecten\bugs\Console\expandfilename>expfn
c:\devel\fpc\\..\ -> C:\devel\fpc\
c:\devel\fpc\\.. -> C:\devel\fpc
c:\devel\fpc\..\ -> C:\devel\
c:\devel\fpc\.. -> C:\devel

C:\Users\Bart\LazarusProjecten\bugs\Console\expandfilename>dcc32 expfn.lpr
Borland Delphi Version 15.0
...
20 lines, 0.02 seconds, 29600 bytes code, 2969 bytes data.

C:\Users\Bart\LazarusProjecten\bugs\Console\expandfilename>expfn
c:\devel\fpc\\..\ -> c:\devel\
c:\devel\fpc\\.. -> c:\devel
c:\devel\fpc\..\ -> c:\devel\
c:\devel\fpc\.. -> c:\devel
Additional InformationSee forum discussion at http://forum.lazarus.freepascal.org/index.php/topic,42155.0.html
Reported there by user ASBzone.
TagsNo tags attached.
Fixed in Revision39840
FPCOldBugId
FPCTarget
Attached Files

Activities

Bart Broersma

2018-08-21 19:39

reporter   ~0110213

Sorry: wrong forumlink.
Must be: http://forum.lazarus.freepascal.org/index.php/topic,42287.msg294940.html#msg294940
Original reporter: comingnine

Bart Broersma

2018-08-21 19:47

reporter   ~0110214

Lazarus has a similar function ExpandFilenameUTF8 (unit LazFileUtils).
There this issue is resolved by replacing a double DirectorySeparator with a single one before parsing the dots.
Wether or not this makes sense may be platform dependant (fpc supports far more platforms than Lazarus).

Tomas Hajny

2018-08-21 22:30

manager   ~0110218

Indeed - technically, the doubled directory separator is simply wrong and the path is invalid. MS Windows is fairly forgiving and ignores this error. It's rather questionable whether this behaviour should be followed in our routine. I'll perform some testing and decide afterwards.

Bart Broersma

2018-08-21 23:02

reporter   ~0110219

Last edited: 2018-08-21 23:16

View 3 revisions

Well Windows simply allows it, Delphi resolves it like above, we don't.
IIRC linux also accepts // in those places.

Remark: ExpandFilename should on Windows not alter a path if it starts with \\?\

writeln('\\?\c:\devel\fpc\.. -> ',expandfilename('\\?\c:\devel\fpc\..'));
outputs (fpc and Delphi)
\\?\c:\devel\fpc\.. -> \\?\c:\devel

According to http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx in a path that uses uses ExtendedLengthPath scheme all characters are a literal part of the path.
Expandfilename should leave it untouched.

"Because it turns off automatic expansion of the path string, the "\\?\" prefix also allows the use of ".." and "." in the path names"

Tomas Hajny

2018-08-21 23:22

manager   ~0110220

As mentioned, I'll test the behaviour of different platforms and decide afterwards. Note that relying on getting the same output for invalid input is not a very good practice to say the least. Regarding Delphi - I believe that Delphi doesn't do anything with it, because it simply uses the respective MS Windows API (and that one ignores invalid paths). We don't do that in order to get consistent behaviour across all platforms (the Delphi / MS Windows routine is obviously the reference for us, but that doesn't imply anything for invalid paths).

Bart Broersma

2018-08-21 23:35

reporter   ~0110222

As long as Windows I/O API's accept paths with double backslashes, why would you say the path is invalid?
Even in the DOS days this worked.
Wiindows I/O API even accepts ridiculous filenames like:
'c:\\users/\bart\\lazarusprojecten//bugs\\console\/expandfilename\\foo.txt'

const
  fn = 'c:\\users/\bart\\lazarusprojecten//bugs\\console\/expandfilename\\foo.txt';

var
  t: textfile;

begin
  assignfile(t,fn);
  rewrite(t);
  writeln(t,'Hello World');
  closefile(t);
end.

Creates the file 'C:\Users\Bart\LazarusProjecten\bugs\Console\expandfilename\foo.txt'

Marco van de Voort

2018-08-22 17:47

manager   ~0110238

(afaik only with backporting windows api to the dos prompt with the LFN apis this was allowed. barebones dos is very picky)

Tomas Hajny

2018-08-22 23:54

manager   ~0110244

@Bart Broersma: Not following the specification is IMHO equal to being invalid; the specification does not mention that multiple directory separators may be used without any intermittent characters (see e.g. https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file).

Bart Broersma

2018-08-23 10:43

reporter   ~0110254

Last edited: 2018-08-23 18:20

View 2 revisions

@marco: IIRC this worked with TP 6 on DOS 5...
@tomas: maybe then document this (since it differs from Delphi)?


Edit: @marco: I remembered wrong. TP6 doesn't do that.

Bart Broersma

2018-08-23 10:47

reporter   ~0110255

On Win platform can't we simply use windows API getfullpathname()? See: https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getfullpathnamew

Wolfgang Lieff

2018-08-23 10:47

reporter   ~0110256

The IEEE 1003.1 (Linux/Unix base specs, retrieved from opengroup.org) definition of a valid pathname explicitly requires multiple separators to be treated as one:

3.266 - Pathname
A character string that is used to identify a file. In the context of IEEE Std 1003.1-2001, a pathname consists of, at most, {PATH_MAX} bytes, including the terminating null byte. It has an optional beginning slash, followed by zero or more filenames separated by slashes. A pathname may optionally contain one or more trailing slashes. Multiple successive slashes are considered to be the same as one slash.

Bart Broersma

2018-08-23 17:58

reporter   ~0110267

On Windows GetFullPathNameW for 'c:\devel\fpc\\..\' returns 'c:\devel\'.

Tomas Hajny

2018-08-23 23:29

manager   ~0110272

@Wolfgang: That is indeed interesting, that may suggest that at least some other platforms may ignore multiple directory separators consistently and thus may be a reason for modifying the behaviour of ExpandFilename. Nevertheless, I'll perform some testing and decide afterwards (probably next week).

@Bart: I believe that a cross-platform common implementation ensuring consistent behaviour on all FPC supported platforms is much better than using an operating system API function for a functionality having no operating system dependencies, thus using GetFullPathName(W) is not a preferred solution (although I understand that this is probably used by Delphi on Win32).

Tomas Hajny

2018-08-23 23:32

manager   ~0110273

BTW (@Bart in another post) - obviously, updating our documentation is indeed an option as well (but it's possible that changing the behaviour may be more appropriate).

Serge Anvarov

2018-08-23 23:46

reporter   ~0110275

However, the file names vary depending on the operating system. So it may be better to trust the system to deal with file names (use platform API)?

Tomas Hajny

2018-08-24 00:00

manager   ~0110276

No, the file names / paths do not vary that much, in fact. Obviously, there are some differences like different characters used as directory separators, etc., but most of it is the same or at least shared by multiple operating system / targets. Moreover, there is no direct equivalent of the respective function on all FPC supported targets. Obviously, using the filename to open a file or something similar needs to be done by the operating system, but this function is different from that, because it performs just translation of strings.

Bart Broersma

2018-08-24 16:42

reporter   ~0110286

@Tomas (0110272 and 0110273): OK with me. I'll wait and see.

Wolfgang Lieff

2018-08-25 04:40

reporter   ~0110299

@Tomas (0110276): yes, most if not all filesystems nowadays are POSIX-based/compliant/tolerant with their respective OS-flavours (eg. '/' and case-insensitivity) added on top (Windows handles POSIX paths correctly since at least NT 3.5 in order to be able to bid for US government contracts which required FIPS151 compliance at the time).

Tomas Hajny

2018-09-13 00:43

manager   ~0110711

Sorry for the delay. After performed testing, I can confirm that in addition to MS Windows (where the behaviour was known to me and confirmed by the results of the test program compiled with Delphi as well), Linux and OS/2 indeed ignore multiple directory separators. In addition, Linux DosEmu and VirtualBox (as other environments allowing to run DOS applications) ignore them as well. I have no MS Windows VDM nor LFN-enabled DOS installation available, but I believe that they would behave similarly.

As already suspected (but tested nevertheless), the current FPC behaviour is compatible to TP/BP running under plain DOS. In spite of this, I believe that we could modify it and allow handling of these paths (not valid under plain DOS) in ExpandFileName and FExpand in accordance to the definition referenced by Wolfgang and the real behaviour of most other currently supported platforms. I'll modify the implementation, test it and provide another update here afterwards.

Tomas Hajny

2018-09-29 00:59

manager   ~0111067

Finally modified as discussed.

Issue History

Date Modified Username Field Change
2018-08-21 19:37 Bart Broersma New Issue
2018-08-21 19:39 Bart Broersma Note Added: 0110213
2018-08-21 19:47 Bart Broersma Note Added: 0110214
2018-08-21 22:25 Tomas Hajny Assigned To => Tomas Hajny
2018-08-21 22:25 Tomas Hajny Status new => assigned
2018-08-21 22:30 Tomas Hajny Note Added: 0110218
2018-08-21 23:02 Bart Broersma Note Added: 0110219
2018-08-21 23:16 Bart Broersma Note Edited: 0110219 View Revisions
2018-08-21 23:16 Bart Broersma Note Edited: 0110219 View Revisions
2018-08-21 23:22 Tomas Hajny Note Added: 0110220
2018-08-21 23:35 Bart Broersma Note Added: 0110222
2018-08-22 17:47 Marco van de Voort Note Added: 0110238
2018-08-22 23:54 Tomas Hajny Note Added: 0110244
2018-08-23 10:43 Bart Broersma Note Added: 0110254
2018-08-23 10:47 Bart Broersma Note Added: 0110255
2018-08-23 10:47 Wolfgang Lieff Note Added: 0110256
2018-08-23 17:58 Bart Broersma Note Added: 0110267
2018-08-23 18:20 Bart Broersma Note Edited: 0110254 View Revisions
2018-08-23 23:29 Tomas Hajny Note Added: 0110272
2018-08-23 23:32 Tomas Hajny Note Added: 0110273
2018-08-23 23:46 Serge Anvarov Note Added: 0110275
2018-08-24 00:00 Tomas Hajny Note Added: 0110276
2018-08-24 16:42 Bart Broersma Note Added: 0110286
2018-08-25 04:40 Wolfgang Lieff Note Added: 0110299
2018-09-13 00:43 Tomas Hajny Note Added: 0110711
2018-09-29 00:59 Tomas Hajny Fixed in Revision => 39840
2018-09-29 00:59 Tomas Hajny Note Added: 0111067
2018-09-29 00:59 Tomas Hajny Status assigned => resolved
2018-09-29 00:59 Tomas Hajny Fixed in Version => 3.3.1
2018-09-29 00:59 Tomas Hajny Resolution open => fixed
2018-09-29 19:11 Bart Broersma Status resolved => closed