View Issue Details

IDProjectCategoryView StatusLast Update
0030766FPCRTLpublic2016-12-04 13:45
ReporterMichalis KamburelisAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformx86-64OSDebian GNU/LinuxOS Version(testing)
Product Version3.1.1Product Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0030766: [patch] On Unix, FileOpen works on directories, which causes weird TFileStream.Size, TMemoryStream.LoadFromFile behaviour
DescriptionOn Linux, opening a directory with TFileStream works. What's worse, TFileStream.Size is then a huge number 9223372036854775807 (at least on 64-bit system). One bad effect of this is that TMemoryStream.LoadFromFile fails with confusing EOutOfMemory exception. Tested with FPC 3.0.0 and latest 3.1.1.

On Windows, opening a directory fails with EFOpenError, and this seems more sane.

So I would propose to change FileOpen (used by TFileStream.Create) in rtl/unix/sysutils.pp to check whether the file is a directory, and then return feInvalidHandle in this case. Although various Unix documentation calls "directory" a "special kind of file", you cannot really read the directory contents using TFileStream, or interpret it's size in any cross-platform useful manner.

This change will be

- compatible with what happens on Windows,

- and make TFileStream.Create and TMemoryStream.LoadFromFile fail with EFOpenError, which seems more reasonable than current behavior.

Attaching a patch that does exactly that.
TagsNo tags attached.
Fixed in Revision35063
FPCOldBugId
FPCTarget
Attached Files
  • test_filestream.lpr (311 bytes)
  • fileopen_fail_on_directory.patch (934 bytes)
    Index: rtl/unix/sysutils.pp
    ===================================================================
    --- rtl/unix/sysutils.pp	(wersja 34745)
    +++ rtl/unix/sysutils.pp	(kopia robocza)
    @@ -443,6 +443,12 @@
     
     Function FileOpenNoLocking (Const FileName : RawbyteString; Mode : Integer) : Longint;
     
    +  Function IsHandleDirectory(Handle : Longint) : boolean;
    +  Var Info : Stat;
    +  begin
    +    Result := (fpFStat(Handle, Info)<0) or fpS_ISDIR(info.st_mode);
    +  end;
    +
     Var
       SystemFileName: RawByteString;
       LinuxFlags : longint;
    @@ -458,6 +464,12 @@
       repeat
         FileOpenNoLocking:=fpOpen (pointer(SystemFileName),LinuxFlags);
       until (FileOpenNoLocking<>-1) or (fpgeterrno<>ESysEINTR);
    +
    +  { Do not allow to open directories with FileOpen.
    +    This would cause weird behavior of TFileStream.Size, 
    +    TMemoryStream.LoadFromFile etc. }
    +  if IsHandleDirectory(FileOpenNoLocking) then
    +    FileOpenNoLocking:=feInvalidHandle;
     end;
     
     
    

Activities

Michalis Kamburelis

2016-10-20 16:33

reporter  

test_filestream.lpr (311 bytes)

Michalis Kamburelis

2016-10-20 16:34

reporter  

fileopen_fail_on_directory.patch (934 bytes)
Index: rtl/unix/sysutils.pp
===================================================================
--- rtl/unix/sysutils.pp	(wersja 34745)
+++ rtl/unix/sysutils.pp	(kopia robocza)
@@ -443,6 +443,12 @@
 
 Function FileOpenNoLocking (Const FileName : RawbyteString; Mode : Integer) : Longint;
 
+  Function IsHandleDirectory(Handle : Longint) : boolean;
+  Var Info : Stat;
+  begin
+    Result := (fpFStat(Handle, Info)<0) or fpS_ISDIR(info.st_mode);
+  end;
+
 Var
   SystemFileName: RawByteString;
   LinuxFlags : longint;
@@ -458,6 +464,12 @@
   repeat
     FileOpenNoLocking:=fpOpen (pointer(SystemFileName),LinuxFlags);
   until (FileOpenNoLocking<>-1) or (fpgeterrno<>ESysEINTR);
+
+  { Do not allow to open directories with FileOpen.
+    This would cause weird behavior of TFileStream.Size, 
+    TMemoryStream.LoadFromFile etc. }
+  if IsHandleDirectory(FileOpenNoLocking) then
+    FileOpenNoLocking:=feInvalidHandle;
 end;
 
 

Michael Van Canneyt

2016-12-04 13:45

administrator   ~0096501

Your patch needed correction: the file handle needed to be closed if it was a directory, otherwise a handle leak occurs.

Thanks for the patch.

Issue History

Date Modified Username Field Change
2016-10-20 16:33 Michalis Kamburelis New Issue
2016-10-20 16:33 Michalis Kamburelis File Added: test_filestream.lpr
2016-10-20 16:34 Michalis Kamburelis File Added: fileopen_fail_on_directory.patch
2016-11-27 13:24 Michael Van Canneyt Assigned To => Michael Van Canneyt
2016-11-27 13:24 Michael Van Canneyt Status new => assigned
2016-12-04 13:45 Michael Van Canneyt Fixed in Revision => 35063
2016-12-04 13:45 Michael Van Canneyt Note Added: 0096501
2016-12-04 13:45 Michael Van Canneyt Status assigned => resolved
2016-12-04 13:45 Michael Van Canneyt Fixed in Version => 3.1.1
2016-12-04 13:45 Michael Van Canneyt Resolution open => fixed
2016-12-04 13:45 Michael Van Canneyt Target Version => 3.2.0