Weird code in THttpConnection.ProcessWellKnownDir

[Kyle_Katarn31 0]

Some observations :

{ V8.65 look for absolute URL sent by proxy }
    I := Pos('://', FPath);
    if (I = 4) or (I = 5) then begin
        FPath := Copy(FPath, I + 3, 99999);  // strip http://
        I := Pos('/', FPath);  // start of path
        if (I > 1) then
            FPath := Copy(FPath, I, 999999);  // strip host
    end;

Magic numbers 99999 and 999999 to be replaced with actual expected FPath extract length ?

 

 else begin
                { see if we have access to file, but don't read it yet }
                TempStream := TFileStream.Create(FDocument, fmOpenRead + fmShareDenyWrite);
                TempStream.Destroy;
                OK := TRUE;
            end;

Destroy is called even if .Create fails and the global code section is a nonsense as always returns OK = TRUE whatever the access rights 

 

TempoStream.Free shall be called rather than TempoStream.Destroy

Edited September 25, 2022 by Kyle_Katarn31

[Brian Evans 105]

It won't copy more characters than are in the string. Passing a large number for count is basically saying copy until the end is reached. 

 

However if the count parameter is left out the compiler will put in max int behind the scenes leading to cleaner code and no problematic magic value that could truncate a really long string.

 FPath := Copy(FPath, I + 3);

[Kyle_Katarn31 0]

2 minutes ago, Brian Evans said:

It won't copy more characters than are in the string. Passing a large number for count is basically saying copy until the end is reached. 

 

However if the count parameter is left out the compiler will put in max int behind the scenes leading to cleaner code and no problematic magic value that could truncate a really long string.

 FPath := Copy(FPath, I + 3);

That's exactly my point.

[David Heffernan 2345]

11 hours ago, Kyle_Katarn31 said:

Destroy is called even if .Create fails

This statement is incorrect. The only way for Create to fail is if it raises an exception. The code is odd, but it doesn't do what you say. 

[Kyle_Katarn31 0]

18 minutes ago, David Heffernan said:

This statement is incorrect. The only way for Create to fail is if it raises an exception. The code is odd, but it doesn't do what you say. 

Yes, right.

Anyway : TempoStream.Free shall be called rather than TempoStream.Destroy

[David Heffernan 2345]

36 minutes ago, Kyle_Katarn31 said:

TempoStream.Free shall be called rather than TempoStream.Destroy

That's what I would do, but it's just a needless nil check which a compiler could remove. 

[Kyle_Katarn31 0]

Ok. So at least the Copy enhancement to be considered ?

[David Heffernan 2345]

1 hour ago, Kyle_Katarn31 said:

Ok. So at least the Copy enhancement to be considered ?

I don't know what you mean here. Did you write this code? What problems are you having with the code? 

[Kyle_Katarn31 0]

Just a comment. No problem. My problem is on the assert in the other thread

[FPiette 383]

1 hour ago, Kyle_Katarn31 said:

Ok. So at least the Copy enhancement to be considered ?

I'm not sure at all that the old Delphi compilers ICS still support have an option 3rd argument.

 

[Kyle_Katarn31 0]

Then argument to be filled with actual needed length

[David Heffernan 2345]

If that is the case, then MaxInt should be used for that argument

[Kyle_Katarn31 0]

MaxInt or expected length to be copied (Length()-xxx)

[David Heffernan 2345]

MaxInt

[Remy Lebeau 1394]

It doesn't really matter what you use, as long as the value is "large enough".  If the value you specify is more than the number of characters actually available, Copy() will just stop when it reaches the end of the string. So, whether you use:

FPath := Copy(FPath, I + 3);

Or

FPath := Copy(FPath, I + 3, MaxInt);

Or

FPath := Copy(FPath, I + 3, Length(FPath));

Or

FPath := Copy(FPath, I + 3, Length(FPath) - I + 3);

It is all the same as far as Copy() cares.  Personally, I use MaxInt.

[Angus Robertson 574]

I too use MaxInt for new applications, but I've no idea whether it is available in all the ancient compilers ICS still supports, so always keep code a simple as possible. 

 

There is no complete list of new features, functions and constants added with each new release,  When I needed to use PosEx to simplify some code, I had to use three conditionals as it changed three times since Delphi 7, had to search old source directories to find out when, which all takes valuable time.

 

This won't matter for much longer since the next major release of ICS will cease support for compilers more than a few years old, so we can finally make use of new language features and remove lots of ANSI specific stuff.  But we will changing too much old working code, since invariably changes introduce errors and often new things don't work properly.

 

For instance when using TSearchRec to index files, I tried using the new TimeStamp property that returns TDateTime instead of Time that returns a file date stamp, but eventually discovered TimeStamp returns UTC time instead of local time, both undocumented in help so don't know if this is deliberate or a bug. 

 

But it meant my applications could not correctly compare source and destination directories when FTP or copying files, causing hundred of thousands of extra files to be copied, until I reverted to using Time.  So new is not always better.

 

Angus

 

 

 

 

[Kyle_Katarn31 0]

Thanks

"This won't matter for much longer since the next major release of ICS will cease support for compilers more than a few years old"

 

You mean that you are about to drop Delphi 7 support ? What a bad news !

 

[David Heffernan 2345]

6 minutes ago, Kyle_Katarn31 said:

What a bad news !

The old code won't stop working. It's unrealistic to expect developers to expend extra time supporting legacy compilers and systems. It's no fun doing that. 

[Lars Fosdal 1792]

Legacy = Age > 4 years 😛

[Angus Robertson 574]

No decision yet on how many old releases the next major version of ICS will support, partly down to age of old new language features, and partly because each time I add a new unit I have to update 100 to 200 .dpr, .dprog and .cprog files, something we should have automated a long time ago, but it's quicker to do it manually each time...

 

ICS V7 will still be supported for new compilers and OpenSSL versions (unless totally breaking) and major bugs, but no new components or features, unless another contributor takes over that support and testing, I've now migrated all my D2007 projects to D11 so don't need to use D2007 any longer. 

 

Angus

 

[FPiette 383]

The new version I started to develop take 90% of existing ICS code. The new code is developed with Delphi 11 and no effort is done to support older compilers. Since I keep Delphi always up-to-date, it is likely that at the time of release, the next ICS version AKA V9 will officially and initially only support the latest Delphi at that time.

[Fr0sT.Brutal 900]

D7 has MaxInt for sure as I use it everywhere in Copy. I really can't understand why Borland didn't make 3rd parameter optional.

That magic constant likely has its origin from the very ancient versions. However, that could be High(Integer) instead...

On 10/3/2022 at 4:28 PM, Lars Fosdal said:

Legacy = Age > 4 years 😛

4 is magic constant defined personally by yourself 😛

Edited October 10, 2022 by Fr0sT.Brutal

[Lars Fosdal 1792]

That's an estimate based on when the average time before the warranty on a laptop runs out and updates are no longer provided 😛