#156 NSISdl http/1.1 support and improvements

[Che Rivera]

Changes:

NSISdl.cpp:
-download function now attempts to resume a failed download. If /RETRIES= is omitted in the install script, download() functions exactly as before.
-arguments to the download function are now order independent. (!!)
-translations can now include most error, resume, and connecting strings.

httpget.cpp:
-now accepts response 206 Partial Content.

ReadMe.txt:
-updated to reflect above changes.

NOTES:
While the "Range:" header is from HTTP/1.1 the servers I have tested this on response correctly with "206 Partial Contents" and don't chunk the data. It works, the only thing remaining unimplemented for full HTTP/1.1 compatibility is chunked transfer encodings. Thats the next phase.

I would like to test this on some other less common servers. IIS and Apache seem to respond to a HTTP/1.0 request with a "Range:" header correctly.

[Amir Szekely]

Logged In: YES
user_id=584402
Originator: NO

Thanks, I'll have a look after 2.30.

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

File Added: nsisdl_patch.diff

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

Changes in Latest Patch:
-All .cpp and .h files received the same header comment layout and changed erroneous license file reference to license.txt.

NSISdl.cpp
-download function now able to do file-based resumes with /RESUME.
-download function will not delete the target file if /NODELETE is specified.
-accepts /FILESIZE= allows the script to specify filesize (content-length header no longer required)

httpget.cpp
-now checks for "Transfers-Encoding: chunked" and dechunks properly
-correctly skips "100 Continue" headers
-sends "Connection: close" during request

connection.cpp
-new function recv_lineb() functions like recv_line() but returns bytes read instead of a flag

util.cpp
-new function myatoi64h() takes a number in acsii hex and converts it to a 64-bit integer

NOTES:
Obviously /RESUME and /NODELETE are meant to be used together.

Example script snippet:

!define DL_FILE_URL "http://www.someserver.com/file"
!define DL_FILE_SIZE 11213948

; We check to see if the download temp file exists
; if it does we check to see if it is the right filesize
; if it is the right file size we go to unpacking
; otherwise we open a message box to ask for resume permission
; if the file doesn't exist we download as normal
IfFileExists $1 +1 DoesntExist
Push $1
Call FileSizeNew ; FROM WIKI
Pop $2
IntCmp $2 ${DL_FILE_SIZE} Success
MessageBox MB_YESNO "The file $1 already exists.$\nDo you want to resume the download?" /SD IDYES IDYES ResumeDL IDNO DoesntExist
ResumeDL:
NSISdl::download /TIMEOUT=25000 /NODELETE /RESUME /RETRIES=8 /FILESIZE=${DL_FILE_SIZE} ${DL_FILE_URL} "$1"
Goto Done
DoesntExist:
NSISdl::download /TIMEOUT=25000 /NODELETE /RETRIES=8 /FILESIZE=${DL_FILE_SIZE} ${DL_FILE_URL} "$1"
Done:
Pop $R0 ;Get the return value

StrCmp $R0 "success" Success
MessageBox MB_OK "Download failed: $R0"
Quit
Success:

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

File Added: nsisdl_patch.diff

[Che Rivera]

Fixed a few errors in readme.txt

[Che Rivera]

Downloader Skeleton (or nsisdl2 test)

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

File Added: nsisdl2_example.nsi

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

File Added: nsisdl2_patch.diff

[Che Rivera]

Adds fixes for progress bar display on Ultramodern UI

[Amir Szekely]

Logged In: YES
user_id=584402
Originator: NO

It seems like there's an off-by-one problem in the content length calculation. When attempting to resume an already complete file, I always get "HTTP/1.1 416 Requested Range Not Satisfiable" from the server which seems to suggest NSISdl is trying to download fileLength+1. In your example, I see this is handled by having the script look at the file size. I think it'd be better if NSISdl handles this, with HEAD or any other method. The user can't always know the file size and it'd also be easier if NSISdl takes the job.

[Che Rivera]

Logged In: YES
user_id=1817904
Originator: YES

Well there is a problem here. I had to remove all content-length checks from the code in the first place to support http/1.1. The specs for http/1.1 say that the content-length header is optional for certain types of transfers such as chunked transfer encodings (basically they come from server-side scripts or caching proxies.) So the download plug-in has to work even if the file length is unknown by the script AND the plug-in itself. I tried to find answers about using HEAD to get a content-length but everything I read suggests that in instances where GET doesn't include a content-length header neither will HEAD for the same resource.

The plug-in simply returns the "HTTP/1.1 416" response because I hope that the script writer would check for such a response if they don't know the file's size beforehand. In instances where the plug-in does know the filesize before hand, I could do the checks and simply return "success" but then I lose the ability to allow the script writer to provide options for the user (such as "Download seems complete, download the file again?") That is why the /resume switch is an option - to allow the script writer the option to determine how to deal with a file that already exists.

The idea was to give the script writer the most options for determining if the download was a success during an earlier instance of the installer. The script writer could do a CRC32 check on the downloaded file or attempt to unpack an archive. The script would overwrite the file if the file is the right size but those steps fail and resume only if the file is too small.

I was trying to think up a good solution to this problem but you can't really be sure that the plug-in knows the file-size or even how many bytes to expect from the server. Even doing a file-size check if the content-length header is provided isn't a solution because it may work on most machines most of the time, but the moment the person is behind an invisible caching proxy, the script won't function correctly (because the content-length header isn't likely to be received.)

Ultimately, the only person with enough knowledge of what should be downloaded is the script writer and they have to do the right kind of checks on the file and status returned by NSISdl to overcome such conditions. The question becomes what should the "blackbox" of a plug-in take care of itself and what should the script writer be allowed control over.

I was thinking that a better solution might be to include a more comprehensive set of macro's for taking care of different error conditions and handling the issue of resume but I would love to hear what you think.

[Amir Szekely]

Logged In: YES
user_id=584402
Originator: NO

Well, a set of macros may be the solution and the brains can move. I don't have a solid idea for the best solution for this. But I do believe the user should never get HTTP response back from NSISdl. And in this case, it'd seem NSISdl should do the file size check as it does know better. It could try a range smaller by one right after getting 416. If that range doesn't fail, it knows the file is already complete and can immediately return "success". Eventually, in the most common case, I'd think the user should just pass a URL with /ENABLE_RESUME or something similar and have NSISdl handle the rest. If the file is completely replaced, redownload. If the file downloaded seem to have stopped in the past, continue downloading. If the server claims there are no more bytes after the last byte of the file, return "success".