Indy10 TIdHTTP gzip issue
lazarus / 38-Networking and Web Programming

----------------------------------------------------------------------
asdf121 · 2018-11-14 20:03

Hello,
recently I noticed that gzip does not work with TIdHTTP anymore. It
just returns garbage.

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program
IndyGZipIssue; //{$DEFINE STATICLOAD_ZLIB}//{$UNDEF
STATICLOAD_ZLIB} {$IFDEF FPC}        {$MODE Delphi}{$ENDIF} uses   
{$IFDEF MSWINDOWS} Windows, {$ENDIF}    {$IFNDEF MSWINDOWS}cthreads,
cmem,{$ENDIF} SysUtils, Classes,                IdHTTP, IdURI,
IdSSLOpenSSL, IdCompressorZLib, IdExceptionCore, StrUtils,
IdSSLOpenSSLHeaders,                IdGlobal, IdZLibHeaders,
IdZLibCompressorBase; Var  IdHTTP: TIdHTTP; 
IdSSLIOHandlerSocketOpenSSL: TIdSSLIOHandlerSocketOpenSSL;  EncURL:
String;  Str: String;  MyText: TStringlist; begin   
writeln('Start!');                EncURL :=
TIdURI.URLEncode('https://indy.fulgan.com/');        
writeln(EncURL);          try    IdSSLOpenSSLHeaders.Load;  except   
on e: EIdOSSLCouldNotLoadSSLLibrary do    begin     
writeln(Format('Loading OpenSSL failed: %s',
[IdSSLOpenSSLHeaders.WhichFailedToLoad]));      exit;    end;    on e:
Exception do    begin      writeln(Format('[EXCEPTION] OpenSSL: %s
%s', [e.ClassName, e.Message]));      exit;    end;  end; 
writeln('OpenSSL loaded!');                 try    IdHTTP :=
TIdHTTP.Create(nil);    try      with IdHTTP do      begin       
HandleRedirects := True;        RedirectMaximum := 5;       
ConnectTimeout := 5000;        ReadTimeout := 5000;         //
compression                try                        Compressor :=
TIdCompressorZLib.Create(IdHTTP);                except              
        on e: Exception do                        begin              
          writeln(Format('[EXCEPTION] TIdCompressorZLib Create: %s',
[e.Message]));                        end;                  end;     
                           if Compressor = nil then                  
    writeln(Format('Compressor is nil!', []));                       
 //Request.AcceptEncoding := 'gzip';               
Request.AcceptEncoding := 'deflate';      end;       // secure
connection      {$IFNDEF UNICODE}        if AnsiStartsText('https',
EncURL) then      {$ELSE}         if StartsText('https', EncURL) then 
    {$ENDIF}      begin                writeln(Format('Using secure
connection!', []));                 try         
IdSSLIOHandlerSocketOpenSSL :=
TIdSSLIOHandlerSocketOpenSSL.Create(IdHTTP);          IdHTTP.IOHandler
:= IdSSLIOHandlerSocketOpenSSL;                 except               
       on e: Exception do                        begin               
         writeln(Format('[EXCEPTION] HTTP GET %s : %s', [EncURL,
e.Message]));                        end;                  end;     
end;         with IdHTTP do        begin          try            Str
:= Get(EncURL);          except            on e: Exception do        
  begin              writeln(Format('HTTP GET: %s failed: %d ec ---
%s.', [EncURL, ResponseCode, ResponseText]));             
writeln(Format('ClassName: %s <--> Exception: %s', [e.ClassName,
e.Message]));            end;          end;                end;   
finally      IdHTTP.Free;    end;  except    on e: Exception do   
begin      writeln(Format('[EXCEPTION] HTTP GET %s : %s', [EncURL,
e.Message]));    end;  end;    MyText:= TStringlist.Create;  try   
MyText.Text := Str;    MyText.SaveToFile('http.txt');  finally   
MyText.Free  end; {try}     writeln('End!');end.
Compiled with:
Code: [Select]Free Pascal Compiler version 3.2.0-beta-r40097
[2018/11/10] for x86_64

fpc -MDelphi -O3 -FuC:\Users\asdf\Downloads\Indy10_5485\Lib\*
C:\Users\asdf\Downloads\http.pas
You can easily test with changing
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
//Request.AcceptEncoding := 'gzip'; <-- does not
workRequest.AcceptEncoding := 'deflate'; <-- works fine
I also tested to define one of
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i //{$DEFINE
STATICLOAD_ZLIB}//{$UNDEF STATICLOAD_ZLIB}but it also has no effect at
all.

Program output is
Code: [Select]Start!
https://indy.fulgan.com/
OpenSSL loaded!
Using secure connection!
End!
As far as I can say it appears also on Linux.

Regards.

PS: Will Indy support brotli sooner or later?

----------------------------------------------------------------------
asdf121 · 2018-11-17 11:15

Seems I've fixed it at least on Windows - got the 32-bit .dll... 
Does the static linking also work with FPC? If yes, how do I do that?
Static linking only works on Windows, no? How can I see if it's static
or dynamic linked?

But the issue on linux still exist, maybe it's related to the change
of SSLDLLVers, Bug fix for HackLoad() and Updating various DLL related
functions where the last one says Quote// TODO: setup this array more
like the SSLDLLVers arrays in the IdSSLOpenSSLHeaders unit...As far as
I can say: with an older Indy rev 5448 it's still working fine.

EDIT:
Why does QuoteTIdCompressorZLib.Create not raise an exception? It
creates the object but when using gzip, the internal check does only
check if Compressor is not nil which is also true if loading zlib
failed -> returning garbage.

----------------------------------------------------------------------
asdf121 · 2018-11-17 20:21

The issue was introduced between Fix for TIdIMAP4Server.ProcessStore()
not processing FLAGS arguments correctly and Updating various DLL
related functions to use THandle instead of HMODULE. With the first
one my old existing code does still works fine, with one of the newer
versions it does not.
I haven't tested all commits between because the ones I tried weren't
able to compile with FPC.

----------------------------------------------------------------------
Remy Lebeau · 2018-11-19 21:01

Quote from: asdf121 on November 14, 2018, 09:03:50 pmrecently I
noticed that gzip does not work with TIdHTTP anymore. It just returns
garbage.

Your test code works fine for me on Windows, at least (I'm not a Linux
user).

Quote from: asdf121 on November 14, 2018, 09:03:50 pm//{$DEFINE
STATICLOAD_ZLIB}
//{$UNDEF STATICLOAD_ZLIB}

(Un)defining that in your own code has no effect on Indy.  It needs to
be (un)defined in Indy's IdCompilerDefines.inc file instead, and then
recompile Indy.

Quote from: asdf121 on November 14, 2018, 09:03:50
pm//Request.AcceptEncoding := 'gzip';
Request.AcceptEncoding := 'deflate';

Note that you cannot force TIdHTTP to use only deflate when a
Compressor is assigned.  If the Compressor is loaded successfully (its
IsReady property is true), TIdHTTP will automatically add gzip and/or
deflate as needed to the Request.AcceptEncoding if they are not
already present, thus allowing the server to choose between them.

Because TIdHTTP auto-sets the Request.AcceptEncoding if the Compressor
is ready, you should not set the AcceptEncoding manually at all, or at
least not for gzip/deflate specifically.  You should let TIdHTTP
enable gzip/deflate for you only if the Compressor is able to handle
them.

That is likely the root cause of your problem.  The Compressor is not
ready (for whatever reason), so TIdHTTP does not update the
Request.AcceptEncoding, so your manual setting gets passed as-is to
the server, which then sends back a compressed response (not garbage)
that TIdHTTP does not auto-decompress for you.

So, either figure out why the Compressor is not loading correctly, or
else leave the Compressor set to nil, set the Request.AcceptEncodings
as needed, and then handle the decompression manually after the
compressed response is returned to you.

Quote from: asdf121 on November 14, 2018, 09:03:50 pmPS: Will Indy
support brotli sooner or later?

Probably not, at least not anytime soon.  But there is nothing
stopping you from implementing your own Compressor class for it, and
then use it with TIdHTTP.  And if you get something working, you can
always submit it for review for a possible future inclusion.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmDoes the static
linking also work with FPC?

I don't know.  It was designed for Delphi.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmIf yes, how do I
do that? Static linking only works on Windows, no?

I suppose as long as FPC can consume Indy's ZLIB obj files, then it
should work, at least on Windows.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmHow can I see if
it's static or dynamic linked?

There is nothing readily exposed to check that at runtime.  But, at
compile-time, you can probably use {$IF DECLARED(...)} to check if the
IdZlibHeaders.pas unit declares the EIdZLibStubError class or not.  It
is declared only when dynamic linking.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmBut the issue on
linux still exist, maybe it's related to the change of SSLDLLVers, Bug
fix for HackLoad() and Updating various DLL related functions

It should not be related, since those changes do not affect how Indy
uses Zlib.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmwhere the last
one says Quote// TODO: setup this array more like the SSLDLLVers
arrays in the IdSSLOpenSSLHeaders unit...
The changes I made for handling OpenSSL DLLs do not apply to ZLib
DLLs.  So, if ZLib was loading correctly before, it should still load
OK now, I did not change that logic.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmWhy does
QuoteTIdCompressorZLib.Create not raise an exception?

Because it is not supposed to, in part because the constructor is not
what loads ZLib at runtime, the TIdCompressorZLib.IsReady property
getter procedure does.

Otherwise, you can pre-load ZLib by calling IdZLibHeaders.Load()
manually before using TIdHTTP, similar to how you are calling
IdSSLOpenSSLHeaders.Load().

However, note that both IdSSLOpenSSLHeaders.Load() and
IdZLibHeaders.Load() do not raise an exception on load failure, they
return Boolean values instead, eg:

Code: [Select]if not IdSSLOpenSSLHeaders.Load then
begin
  writeln(Format('Loading OpenSSL failed: %s',
[IdSSLOpenSSLHeaders.WhichFailedToLoad]));
  exit;
end;
writeln('OpenSSL loaded!');

if not IdZLibHeaders.Load then
begin
  writeln('Loading ZLib failed');
  // there is no WhichFailedToLoad() function for ZLib!  Individual
ZLib functions will
  // raise an EIdZLibStubError if they cannot be loaded from the DLL
when called...
  exit;
end;
writeln('ZLib loaded!');

Quote from: asdf121 on November 17, 2018, 12:15:47 pmwhen using gzip,
the internal check does only check if Compressor is not nil which is
also true if loading zlib failed -> returning garbage.

TIdHTTP checks the Compressor.IsReady property before updating the
Request.AcceptEncoding, or decompressing a compressed response.

----------------------------------------------------------------------
asdf121 · 2018-11-20 20:38

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmQuote from:
asdf121 on November 14, 2018, 09:03:50 pmrecently I noticed that gzip
does not work with TIdHTTP anymore. It just returns garbage.

Your test code works fine for me on Windows, at least (I'm not a Linux
user).
Yes, on windows it's working fine for me too now - was just an issue
with 32/64bit zlib.

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmQuote from:
asdf121 on November 14, 2018, 09:03:50 pm//{$DEFINE STATICLOAD_ZLIB}
//{$UNDEF STATICLOAD_ZLIB}

(Un)defining that in your own code has no effect on Indy.  It needs to
be (un)defined in Indy's IdCompilerDefines.inc file instead, and then
recompile Indy.
It's only defined in IdZLibConst and IdZLibHeaders, seems it's does
even not support FPC at all. First statement is always $UNDEF.
Btw, why are the included zlib linking files so ancient?

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmQuote from:
asdf121 on November 14, 2018, 09:03:50 pm//Request.AcceptEncoding :=
'gzip';
Request.AcceptEncoding := 'deflate';

Note that you cannot force TIdHTTP to use only deflate when a
Compressor is assigned.  If the Compressor is loaded successfully (its
IsReady property is true), TIdHTTP will automatically add gzip and/or
deflate as needed to the Request.AcceptEncoding if they are not
already present, thus allowing the server to choose between them.

Because TIdHTTP auto-sets the Request.AcceptEncoding if the Compressor
is ready, you should not set the AcceptEncoding manually at all, or at
least not for gzip/deflate specifically.  You should let TIdHTTP
enable gzip/deflate for you only if the Compressor is able to handle
them.

That is likely the root cause of your problem.  The Compressor is not
ready (for whatever reason), so TIdHTTP does not update the
Request.AcceptEncoding, so your manual setting gets passed as-is to
the server, which then sends back a compressed response (not garbage)
that TIdHTTP does not auto-decompress for you.

So, either figure out why the Compressor is not loading correctly, or
else leave the Compressor set to nil, set the Request.AcceptEncodings
as needed, and then handle the decompression manually after the
compressed response is returned to you.
Okay, I'll remove my Request.AcceptEncodings line and let TIdHTTP
decide.

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmQuote from:
asdf121 on November 17, 2018, 12:15:47 pmBut the issue on linux still
exist, maybe it's related to the change of SSLDLLVers, Bug fix for
HackLoad() and Updating various DLL related functions

It should not be related, since those changes do not affect how Indy
uses Zlib.

Quote from: asdf121 on November 17, 2018, 12:15:47 pmwhere the last
one says Quote// TODO: setup this array more like the SSLDLLVers
arrays in the IdSSLOpenSSLHeaders unit...
The changes I made for handling OpenSSL DLLs do not apply to ZLib
DLLs.  So, if ZLib was loading correctly before, it should still load
OK now, I did not change that logic.
Well, you changed the function to load a library which is also used
for zlib. I only can tell you what I noticed, with older Indy version
it loads it correctly and with a newer one TIdHTTP fails to decompress
gzip but uses gzip (own hardcoded Request.AcceptEncodings removed). I
don't know where to search exactly but there must be an issue
somewhere.

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmHowever, note
that both IdSSLOpenSSLHeaders.Load() and IdZLibHeaders.Load() do not
raise an exception on load failure, they return Boolean values
instead, eg:
Thanks for the hint, didn't knew before. Only noticed that
IdSSLOpenSSLHeaders.Load() raises an exception when OpenSSL library
not found. Guessed IdZLibHeaders.Load() would do the same.

Quote from: Remy Lebeau on November 19, 2018, 10:01:14 pmQuote from:
asdf121 on November 17, 2018, 12:15:47 pmwhen using gzip, the internal
check does only check if Compressor is not nil which is also true if
loading zlib failed -> returning garbage.

TIdHTTP checks the Compressor.IsReady property before updating the
Request.AcceptEncoding, or decompressing a compressed response.
I'll test again with checks for Compressor.IsReady which should be
true, else it shouldn't ask for gzip at all, correct? And without gzip
enabled and zlib not working, I shouldn't receive garbage...

----------------------------------------------------------------------
Remy Lebeau · 2018-11-23 23:49

Quote from: asdf121 on November 20, 2018, 09:38:41 pmIt's only defined
in IdZLibConst and IdZLibHeaders, seems it's does even not support FPC
at all. First statement is always $UNDEF.

Well, then I guess that answers your earlier question about whether
FPC is supported.  Static linking is apparently setup only for
Delphi.  If you have tweaks that will work for FPC, feel free to
submit them for review.

Quote from: asdf121 on November 20, 2018, 09:38:41 pmBtw, why are the
included zlib linking files so ancient?

Because nobody has gotten around to updating them.  I can't compile
the zlib obj files myself.  And Indy was never updated to take
advantage of Delphi's or FPC's own zlib wrappers (it is still on the
todo list).

Quote from: asdf121 on November 20, 2018, 09:38:41 pmOkay, I'll remove
my Request.AcceptEncodings line and let TIdHTTP decide.

Please do.

Quote from: asdf121 on November 20, 2018, 09:38:41 pmWell, you changed
the function to load a library which is also used for zlib.

I only can tell you what I noticed, with older Indy version it loads
it correctly and with a newer one TIdHTTP fails to decompress gzip but
uses gzip (own hardcoded Request.AcceptEncodings removed). I don't
know where to search exactly but there must be an issue somewhere.

Again, the changes I made for how OpenSSL is loaded should not have
any effect on how ZLib is loaded.  You are going to have to be more
specific about how you think the loading logic has changed for ZLib
between the older version to the newer version.  Step through the code
with the debugger and see exactly how each version loads the DLL. I
have no way of testing or debugging it myself, as I don't use Linux or
FPC.

Quote from: asdf121 on November 20, 2018, 09:38:41 pmThanks for the
hint, didn't knew before. Only noticed that IdSSLOpenSSLHeaders.Load()
raises an exception when OpenSSL library not found.
No, higher-level code (in
TIdSSLIOHandlerSocketOpenSSL.SetPassThrough() and
TIdSSLContext.Create()) is what raises the exception if
IdSSLOpenSSLHeaders.Load() fails.  Load() itself does not raise the
exception, and never has.  Same with IdZLibHeaders.Load().

Quote from: asdf121 on November 20, 2018, 09:38:41 pmI'll test again
with checks for Compressor.IsReady which should be true, else it
shouldn't ask for gzip at all, correct?

If you leave out gzip and deflate from the Request.AcceptEncoding
property, then yes.  If you do not manually request compression, and
the Compressor is not ready, then TIdHTTP will not request
compression.

Quote from: asdf121 on November 20, 2018, 09:38:41 pmAnd without gzip
enabled and zlib not working, I shouldn't receive garbage...

It is not garbage, it is compression.  There is a difference.

----------------------------------------------------------------------
Mo0211 · 2018-11-28 21:10

Hey Remy,

i analyzed this behavior and got it working on FPC:

there are several issues in this case:

First one in hackload and LoadLibVer functions:

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i ZLib:
TLibHandle = 0;
the return-type on linux FPC is a TLibHandle-Object.
So, the return kinda overflows in your code (value of
-40XXXXXXXXXXXX).

The next issue is the Handling in IdZlibHeaders:

The loading of libz-library is dynamic implemented.
But it checks of Handle-Value of <> 0 - if the Value owerflowed then
it has a high negative value.

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i   if
High(ALibVersions) > -1 then begin    Result := NilHandle;    for i :=
Low(ALibVersions) to High(ALibVersions) do    begin      Result :=
LoadLibVer(ALibVersions[i]);      if Result > NilHandle then begin   
   Break;      end;    end;  end else begin    Result :=
LoadLibVer('');
i'm not sure if the dynamically loading works - i tried to change it
in loading only ''.

the library is named libz per default in linux (not zlib).

I hope this is a help for you to review and fix it

Thanks and good evening!

edit: Forgot to mention i use ubuntu Linux 18.04 64 Bit and fpc
version 3.3.1.

----------------------------------------------------------------------
Remy Lebeau · 2018-11-28 23:54

Quote from: Mo0211 on November 28, 2018, 10:10:52 pmFirst one in
hackload and LoadLibVer functions:

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i ZLib:
TLibHandle = 0;
the return-type on linux FPC is a TLibHandle-Object.
So, the return kinda overflows in your code (value of
-40XXXXXXXXXXXX).

That is not what the declaration looks like in the current version of
IdZLibHeaders.pas:

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i hZLib:
THandle = 0;
All uses of HMODULE (which FPC defines as an alias for TLibHandle)
were replaced with THandle several weeks ago.  Make sure you are using
the latest revision.

Quote from: Mo0211 on November 28, 2018, 10:10:52 pmThe next issue is
the Handling in IdZlibHeaders:

The loading of libz-library is dynamic implemented.
But it checks of Handle-Value of <> 0 - if the Value owerflowed then
it has a high negative value.

That is not a problem in the latest version.  Besides, even if it did
overflow, it still would not be 0.

Quote from: Mo0211 on November 28, 2018, 10:10:52 pmCode: Pascal
 [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i   if
High(ALibVersions) > -1 then begin    Result := NilHandle;    for i :=
Low(ALibVersions) to High(ALibVersions) do    begin      Result :=
LoadLibVer(ALibVersions[i]);      if Result > NilHandle then begin   
   Break;      end;    end;  end else begin    Result :=
LoadLibVer('');
That is not what the latest loop code looks like.  Although yes, the
code was still using NilHandle (I just now fixed that to use 0
instead), the comparison has never used > , it has always used <>.

Quote from: Mo0211 on November 28, 2018, 10:10:52 pmthe library is
named libz per default in linux (not zlib).

'libz' is the base name that Indy uses to load ZLib dynamically on all
UNIX platforms, including Linux:

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i   {$IFDEF
UNIX}const  //The extensions will be resolved by IdGlobal.HackLoad 
//This is a little messy because symbolic links to libraries may not
always be the same  //in various Unix types.  Even then, there could
possibly be differences.  libzlib = 'libz';  // TODO: setup this array
more like the SSLDLLVers arrays in the IdSSLOpenSSLHeaders unit... 
libvers : array [0..3] of string = ('.1','','.3','.2');  {$ENDIF}

----------------------------------------------------------------------
Mo0211 · 2018-11-29 06:49

Hi Remy,

sorry for my misleading comments and first of all thank you for your
fast reply.

All code i posted is already "corrected" by me.

you are right, the ZLib-Object is defined as THandle.
But issue is that when the library will we loaded and the return-value
will be given back a conversion from TLibHandle to THandle does not
work.
And this seems to be the main issue for me.

Thats my i changed the object back to TLibHandle which is working.

Also checking for the result with nilhandle was changed by me in the
way that i compare with bigger then because when the object has
overflowed then the number is a high negative value.
So, the value seems only be correct to me if a big positive number is
assigned.

i have to check further if the dynamicaly loading works with this
setup.
I will come back to you with this information.

But the loading of the lib does not work correctly in this time on
fpc.

----------------------------------------------------------------------
asdf121 · 2018-11-29 09:28

Good to see that I'm not the only one who has issues lately. 

Quote from: Remy Lebeau on November 29, 2018, 12:54:52 amAll uses of
HMODULE (which FPC defines as an alias for TLibHandle) were replaced
with THandle several weeks ago.

After reading the documentation from FPC, it seems that THandle is
just for backward compatibility and always a 32bit signed integer
(which seems to overflow on 64bit platform as reported above).
Whereas TLibHandle is defined differently on various platforms. Used
LoadLibrary function does also return a TLibHandle.

In the documentation for HMODULE (Windows compatibility type), you see
that it's defined as unsigned 32-bits integer (when using
SafeLoadLibrary function - might be problematic on 64bit but I guess
it's Windows only).

I guess the following code in IdGlobal.pas is also problematic, even
if NO_REDECLARE is defined on FPC as default but if you undef it, I
assume it'll fail on 64bit too.
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i  {$IFNDEF
NO_REDECLARE}    {$IFDEF LINUX}      {$IFNDEF VCL_6_OR_ABOVE}  THandle
= UInt32; //D6.System <-- should be platform independent     
{$ENDIF}    {$ENDIF}{$ENDIF}

----------------------------------------------------------------------
asdf121 · 2018-11-29 20:00

Small program to test bytes of each type:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program
SizeOfTest; uses    SysUtils, DynLibs; begin   
writeln(Format('TLibhandle bytes %d', [SizeOf(TLibhandle)]));   
writeln(Format('THandle bytes %d', [SizeOf(THandle)]));   
writeln(Format('HMODULE bytes %d', [SizeOf(HMODULE)]));end. 
Free Pascal Compiler version 3.0.4 [2018/04/10] for x86_64:
Code: [Select]TLibhandle bytes 8
THandle bytes 4
HMODULE bytes 8

Free Pascal Compiler version 3.1.1 [2018/02/09] for x86_64:
Code: [Select]TLibhandle bytes 8
THandle bytes 4
HMODULE bytes 8

Free Pascal Compiler version 3.2.0-beta-r40301 [2018/11/26] for
x86_64:
Code: [Select]TLibhandle bytes 8
THandle bytes 4
HMODULE bytes 8

----------------------------------------------------------------------
Remy Lebeau · 2018-12-01 20:28

Quote from: asdf121 on November 29, 2018, 10:28:57 amAfter reading the
documentation from FPC, it seems that THandle is just for backward
compatibility and always a 32bit signed integer (which seems to
overflow on 64bit platform as reported above).

Then that is an incompatibility with Delphi (and Indy uses FPC in
Delphi mode), whose System.THandle is a NativeUInt, so it is 4 bytes
on 32bit platforms and 8 bytes on 64bit platforms.

Quote from: asdf121 on November 29, 2018, 10:28:57 amIn the
documentation for HMODULE (Windows compatibility type), you see that
it's defined as unsigned 32-bits integer (when using SafeLoadLibrary
function - might be problematic on 64bit but I guess it's Windows
only).

FPC defines System.HMODULE as a PtrUInt, so it is 8 bytes on a 64bit
platform.

Quote from: asdf121 on November 29, 2018, 10:28:57 amI guess the
following code in IdGlobal.pas is also problematic, even if
NO_REDECLARE is defined on FPC as default but if you undef it, I
assume it'll fail on 64bit too.

That code is intended for Delphi only, not FPC.

Indy was using HMODULE originally, but apparently there is an
incompatibility with HMODULE caused by FPC's DynLibs unit (which a few
of Indy's units do use), that is why I switched everything to
THandle.  But I did not know that FPC limits THandle to 4 bytes even
on 64bit platforms (Delphi doesn't).

Looks like I have some more fixes to make for FPC 

Update: Actually, I just now looked at the latest FPC 3.0.4 RTL
source, and there is no single place where THandle is defined. 
Different platforms define their own THandle, where some define it as
4 bytes, and some define it as 8 bytes (Win64 does, for instance). 
So, it is actually a platform-specific type.  But maybe some platforms
are not defining it as 8 bytes for x64 correctly?  What platform are
you compiling your SizeOfTest project on?  I see the RTL using THandle
all over the place.  So, I'm a little lost whether I should keep Indy
using THandle, or have it go back to using HMODULE/TLibHandle and just
work around the DynLibs.HModule incompatibility.

----------------------------------------------------------------------
asdf121 · 2018-12-01 22:13

Quote from: Remy LebeauWhat platform are you compiling your SizeOfTest
project on?
For doing the test with 3.0.4 I used an online compiler.
The other ones were done on Linux Mint 64bit.

I'd use TLibHandle as that is also the return type for LoadLibrary.

I even don't know where THandle is defined. Strange

EDIT:
Quote from: Marco van de VoortThandle is mostly used for filehandles
in the RTL (e.g. the THandleStream, and the COM streamwrappers), which
are 32-bit.
The problem is simply that (really) all handles are THandle on
Windows, but on *nix there many different types.
Changing thandle there, will only cause problems elsewhere.
Use an own defined handle type instead of the generic one.
In general, I would advise you to avoid the use of Windows
compatibility types as much as possible.
https://bugs.freepascal.org/view.php?id=21669

----------------------------------------------------------------------
Remy Lebeau · 2018-12-02 02:11

Quote from: asdf121 on December 01, 2018, 11:13:48 pmI'd use
TLibHandle as that is also the return type for LoadLibrary.

I have just now checked in an update to Indy's SVN so that Indy now
uses TLibHandle on FreePascal again, and keeps using THandle on
Delphi.

Quote from: asdf121 on December 01, 2018, 11:13:48 pmI even don't know
where THandle is defined. Strange

It is defined in various platform-specific .inc files, usually named
sysosh.inc.

Quote from: asdf121 on December 01, 2018, 11:13:48 pmQuote from: Marco
van de VoortThandle is mostly used for filehandles in the RTL (e.g.
the THandleStream, and the COM streamwrappers), which are 32-bit.
The problem is simply that (really) all handles are THandle on
Windows, but on *nix there many different types.
Changing thandle there, will only cause problems elsewhere.
Use an own defined handle type instead of the generic one.
In general, I would advise you to avoid the use of Windows
compatibility types as much as possible.
https://bugs.freepascal.org/view.php?id=21669

Hmm.  There is still quite a bit of THandle usage in Indy beyond just
loading libraries.  I'll review them and see if they need to be
adjusted.  I think the remaining usages are all on Windows, though, so
I can probably just use HMODULE and/or other appropriate handle types.

----------------------------------------------------------------------
asdf121 · 2018-12-03 17:47

Thanks Remy, I've tested your changes and it's working flawless again