Problem with large data returns with Winsock

[Disiance]

I'm trying to include an auto-update feature in my app. Basically, the app downloads a small EXE, which it then runs. The new EXE is the updater itself, which then downloads the program updates and installs them. This updater then restarts the program.

I'm having no problems whatsoever downloading the updater EXE (~66kb), but the actual program update (~366kb) is sometimes corrupt on some machines. The server ends up closing the connection before all of the data is received. A test I just ran on one of the machines messing up the download ended with these results:
1st Try: ~355kb downloaded
2nd Try: ~347kb downloaded

Anyone know why this may be happening? (code is below)


Option Explicit

Private Declare Sub Sleep Lib "kernel32" (ByVal dwMilliseconds As Long)
Private Declare Function MoveFile Lib "kernel32" Alias "MoveFileA" (ByVal lpExistingFileName As String, ByVal lpNewFileName As String) As Long

Private responseBuffer As String

Private Sub Form_Load()
frmMain.Show

lblCaption.Caption = "Initializing"
Sleep 500
lblCaption.Caption = "Connecting to server"

DownloadFiles
End Sub

Private Sub DownloadFiles()
'Connect
sckConnect.Connect
DoEvents
Do Until sckConnect.State = sckConnected Or sckConnect.State = sckError
DoEvents
Loop

'Check for error
If sckConnect.State = sckError Then
MsgBox "There was an error connecting."
sckConnect.Close
DoEvents
End
End If

lblCaption.Caption = "Downloading file 1 of 1"

Dim dlFile As String

'Request update file
sckConnect.SendData "GET /updates/1_4/program.exe HTTP/1.1" & vbCrLf
sckConnect.SendData "Accept: *.*" & vbCrLf
sckConnect.SendData "User-Agent: Autoupdate" & vbCrLf
sckConnect.SendData "Host: www.myhost.com" & vbCrLf
sckConnect.SendData "Connection: close" & vbCrLf
sckConnect.SendData vbCrLf

Do Until sckConnect.State = sckClosed
DoEvents
Loop

'Check for non-200 response
If InStr(1, responseBuffer, "HTTP/1.1 200") = 0 Then
MsgBox "File not found."
End
End If

'Parse file
Dim contentLengthStart As Integer
Dim fileLength As Double
contentLengthStart = InStr(1, responseBuffer, "Content-Length:") + 15
fileLength = Int(Mid(responseBuffer, contentLengthStart, InStr(contentLengthStart, responseBuffer, Chr(10)) - contentLengthStart))

Dim appPath As String
appPath = App.Path
If Not Right(appPath, 1) = "\" Then appPath = appPath & "\"

If Len(Dir(appPath & "program_exe.bak")) > 0 Then Kill appPath & "program_exe.bak"
MoveFile appPath & "program.exe", appPath & "program_exe.bak"

Open appPath & "program.exe" For Output As #1
Print #1, Right(responseBuffer, fileLength)
Close #1

pbProgress.Value = pbProgress.Max

MsgBox "Update complete."

Shell appPath & "program.exe"

End
End Sub

Private Sub sckConnect_Close()
sckConnect.Close
DoEvents
End Sub

Private Sub sckConnect_DataArrival(ByVal bytesTotal As Long)
Dim dataBuffer As String
sckConnect.GetData dataBuffer
responseBuffer = responseBuffer & dataBuffer

pbProgress.Value = Len(responseBuffer)
End Sub

[Lozzenger]

Based on the code it appears that you're trying to retrieve the whole file in one go. I believe there's a limit to the size of the data packet you can send at once

Would it not be better to have you're application download the file in chunks of data at a time?

[Disiance]

It is downloading in smaller chucks, if you notice, the Winsock's DataArrival event builds the file's variable as it's downloaded. I know the size of the String data type isn't the problem, because this code works most of the time.

[Lozzenger]

Well if its only happening occasionally on some machines then it may be an error in the communication (dropping packets or missing data)

Do the problem computers always fail to download the entire file or are they successful at least once?

[Disiance]

They have been successful at times, though it is rare.

I'm wondering... could it be that the connection is being closed before the Winsock sees the last packet? TCP shouldn't allow that to happen, but is it a possibility?

[dilettante]

Wild guess: lost/misprocessed/re-entrant events.

It'd be interesting to know what part of the file is being lost. The beginning? End? Parts in the middle?

Receiving binary data into String variables can present risks as well, particularly corruption due to ANSI/Unicode conversions. This will be worse when the Locale is not EN-US (though EN-GB may be fine too).

The String concatenation can also be problematic.

But I'd guess your use of DoEvents() calls is what is throwing things off here. You're fighting VB hard trying to use the Winsock control in this pseudo-blocking manner.

[ccoder]

I have to agree with dilettante. Get rid of the DoEvents and use the sckConnect events to drive the code. I too would recommend using a byte array as my receive buffer. I can post some code for that if you need it.

Here is an example of how I would write the code to use the sckConnect events. Note: I hacked this in NotePad and visually code checked it but there still may be some typos.

Option Explicit

Private Declare Sub Sleep Lib "kernel32" (ByVal dwMilliseconds As Long)
Private Declare Function MoveFile Lib "kernel32" Alias "MoveFileA" (ByVal lpExistingFileName As String, ByVal lpNewFileName As String) As Long

Private responseBuffer As String

Private Sub Form_Load()
frmMain.Show

lblCaption.Caption = "Initializing"
Sleep 500
lblCaption.Caption = "Connecting to server"
'Connect
sckConnect.Connect

End Sub

Private Sub sckConnect_Connect()
lblCaption.Caption = "Downloading file 1 of 1"

'Request update file
sckConnect.SendData "GET /updates/1_4/program.exe HTTP/1.1" & vbCrLf & _
"Accept: *.*" & vbCrLf & _
"User-Agent: Autoupdate" & vbCrLf & _
"Host: www.myhost.com" & vbCrLf & _
"Connection: close" & vbCrLf & vbCrLf
End Sub

Private Sub sckConnect_Error(ByVal Number As Integer, Description As String, ByVal Scode As Long, ByVal Source As String, ByVal HelpFile As String, ByVal HelpContext As Long, CancelDisplay As Boolean)
' Display the error & close the socket if necessary

MsgBox "There was an error connecting." & vbCRLF & _
"Error " & Number & vbCrLf & Description, vbOKOnly, Me.Caption
If sckConnect.State <> sckClosed Then sckConnect.Close

End Sub

Private Sub sckConnect_SendComplete()
' The SendData has completed

lblCaption.Caption = "Send completed"

End Sub

Private Sub sckConnect_DataArrival(ByVal bytesTotal As Long)
Dim dataBuffer As String
sckConnect.GetData dataBuffer
responseBuffer = responseBuffer & dataBuffer

pbProgress.Value = Len(responseBuffer)
End Sub

Private Sub sckConnect_Close()
Dim dlFile As String
' server has finished download and closed the connection
sckConnect.Close

'Check for non-200 response
If InStr(1, responseBuffer, "HTTP/1.1 200") = 0 Then
MsgBox "File not found."
End
End If

'Parse file
Dim contentLengthStart As Integer
Dim fileLength As Double
contentLengthStart = InStr(1, responseBuffer, "Content-Length:") + 15
fileLength = Int(Mid(responseBuffer, contentLengthStart, InStr(contentLengthStart, responseBuffer, Chr(10)) - contentLengthStart))

Dim appPath As String
appPath = App.Path
If Not Right(appPath, 1) = "\" Then appPath = appPath & "\"

If Len(Dir(appPath & "program_exe.bak")) > 0 Then Kill appPath & "program_exe.bak"
MoveFile appPath & "program.exe", appPath & "program_exe.bak"

Open appPath & "program.exe" For Output As #1
Print #1, Right(responseBuffer, fileLength)
Close #1

pbProgress.Value = pbProgress.Max

MsgBox "Update complete."

Shell appPath & "program.exe"

End
End Sub

[Disiance]

Hmm, thanks for the ideas, both of you! I am very inclined to believe the DoEvents() call is behind this.

ccoder, I am not used to byte arrays, so that example code you spoke of would be appreciated, thanks.

[ccoder]

I posted an example of using a byte array for the Data_Arrival buffer some time back. You can find it here (http://www.vbforums.com/showthread.php?t=397646&highlight=udt).

This example actually uses 2 byte arrays; gbIOBuff to capture the current data and gbReplyBuff to hold all data returned. The CopyMemory API is used to move gbIOBuff to gbReplyBuff. CopyMemory is also used to move gbReplyBuff to the various UDTs. The variable copyPtr is used as a quasi-pointer to the position in gbReplyBuff that gbIOBuff is moved to. Note that both byte arrays are Dimmed 1 byte larger than needed (due to the default "0 to") - this keeps the coding simpler. I can elaborate on this if needed.

There are a number of other things going on in this code that have no bearing on the use of byte arrays, so I won't go into them unless you ask.