Synapse and string problems with HTTPSend in Delphi 2010

Question

I have been trying to get to the bottom of this problem off and on for the past 2 days and I'm really stuck. Hopefully some smart folks can help me out.

The issue is that I have a function that I call in a thread that downloads a file (using Synapse libraries) from a website that is passed to it. However, I've found that every once in a while there are sites where it will not pull down a file, but wget or Firefox/IE will download it without issue.

Digging into it, I've found some curious things. Here is the relevant code:

uses
//[..]
  HTTPSend,
  blcksock;

//[..]

type
  TMyThread = class(TThread)
  protected
    procedure Execute; override;
  private
    { Private declarations }
    fTheUrl: string;
    procedure GetFile(const TheUrl: string);
  public
    property thrd_TheUrl: string read fTheUrl write fTheUrl;
  end;

implementation

[..]

procedure TMyThread.GetFile(const TheUrl: string);
var
  HTTP: THTTPSend;
  success: boolean;
  sLocalUrl: string;
  IsSame : boolean;
begin

  HTTP := THTTPSend.Create;
  try
    HTTP.UserAgent :=
      'Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 2.0.50727)';
    HTTP.ProxyHost := 'MYPROXY.COM';
    HTTP.ProxyPort := '80';

    sLocalUrl :=
      'http://web.archive.org/web/20071212205017/energizer.com/usbcharger/download/UsbCharger_setup_V1_1_1.exe';


   IsSame :=  SameText(sLocalUrl, sTheUrl); //this equals True when I debug

   ///
   ///
   /// THIS IS WHERE THE ISSUE BEGINS
   ///  I will comment out 1 of the following when debugging
   ///
    HTTP.HTTPMethod('GET', sLocalUrl); // ----this works and WILL download the file
    HTTP.HTTPMethod('GET', sTheUrl);  // --- this always fails, and HTTP.ResultString contains "Not Found"

    success := SysUtils.UpperCase(HTTP.ResultString) = 'OK';


    if HTTP.ResultCode > 0 then
      success := True; //this is here just to keep the value around while debugging
  finally
    HTTP.Free;
  end;
end;

procedure TMyThread.Execute
begin
   //fTheURL contains this value:  http://web.archive.org/web/20071212205017/energizer.com/usbcharger/download/UsbCharger_setup_V1_1_1.exe

   GetFile(fTheUrl);
end;

The problem is that when I assign a local variable to the function and give it the URL directly, everything works. However, when passing the variable into the function, it fails. Anyone have any ideas?

    HTTP.HTTPMethod('GET', sLocalUrl); // ----this works and WILL download the file
    HTTP.HTTPMethod('GET', sTheUrl);  // --- this always fails, and HTTP.ResultString contains "Not Found"

I'm using the latest version of Synapse from their SVN repository (version from 2 days ago).

NOTE: The file I am attempting to download is known to have a virus, the program I am writing is meant to download malicious files for analysis. So, don't execute the file once you download it.

However, I'm using this URL b/c this is the one I can reproduce the issue with.

Answer 1

Your code is missing the crucial detail how you use TMyThread class. However, you write

every once in a while there are sites where it will not pull down a file, but wget or Firefox/IE will download it without issue.

which sounds like a timing issue.

Using the local variable works every time. Using the function parameter works only some of the time. That may be caused by the function parameter not containing the correct URL some of the time.

You need to be aware that creating a non-suspended thread may result in it starting to execute immediately (and possibly even to complete), before the next line after the construction call has even started to execute. Setting any property of the thread object after the thread has been created may therefore not work, as the thread execution may be past the point where the property is read. The fTheUrl field of the thread object will be an empty string initially, so whether the thread downloads the file will depend on it being set before.

Your fTheUrl field isn't even protected by a synchronization primitive. Both the thread proc and the code in the main thread can access it concurrently. Sharing data in this way between threads is an unsafe thing to do and may result in any thing from wrong behaviour to actual crashes.

If your thread is really used to download a single file you should remove the write access to the property, and write a custom constructor with a parameter for the URL. That will properly initialize the field before the thread starts.

If you are downloading several files in your program you should really not create a thread for each. Use a pool of threads (may be only one even) that will be assigned the files to download. For that a thread property is the right solution, but then it will need to be implemented with synchronization, and the thread should block when no file is to be downloaded, and unblock when the property is set. The download thread (or threads) would be consumer(s) in a producer-consumer implementation. Stack Overflow has questions and answers regarding this in the Delphi tag, in particular in the questions where alternatives to Suspend() and Resume() are discussed.

One last thing: Don't let unhandled exceptions escape the Execute() method. I'm not sure about whether Delphi 2010 handles these exceptions in the VCL, but unhandled exceptions in a thread may lead to problems like app crashes or freezes.

Answer 2

Well, I'm almost embarrassed to report it, but I owe it to those that took the time to respond.

The issue had nothing to do with Synapse, or TThread, but instead had everything to do with the fact that the URL is case-sensitive!

In my full application, I had a helper function that lowercased the URL (for some reason). I removed that and everything started working again...

Answer 3

Please update last Synapse Revision 127.