Just some details about how this is done in Wolf: Enemy Territory:

libwww (http://www.w3.org/Library/) is used

cvars are:
set sv_wwwDownload
  1 for on 0 for off

set sv_wwwBaseURL
  The prefix for the URL the file exists at.  fs_game is appended to this.
  For example setting this "http://et.tjw.org" may look for the file
  "http://et.tjw.org/etpub/stalingrad.pk3"

set sv_wwwCheckPath
  I think this can be used to specify the local path to the HTTP root to
  verify the pk3s if running the server locally.  Nobody uses this.

set sv_wwwDlDisconnected
  This will disconnect somone from the server when their HTTP/FTP download
  starts.  They will connect again when it finishes.  This is recommended.

I am accepting this bug with the caveat that I'd like it to be as similar as possible to ET in at least cvar name and variable functionality, though obviously I don't expect a 1:1 implementation.

(In reply to comment #1)
> libwww (http://www.w3.org/Library/) is used

I was wrong about that.  It appears that ET uses curl not libwww.  Here's an exerpt from my apache logs:

"GET /etpub/tjw.pk3 HTTP/1.1" 200 489 "ET://72.36.226.186:27960" "ID_DOWNLOAD/2.0 libcurl/7.12.2" "-"

Created attachment 1035 [details]
Ugly but working patch

This patch adds support for download redirection. It is compatible with protocol 68 (therefore the ugly command interception in cl_parse.c/CL_ParseCommandString). It is also compatible with non-capable clients and servers, where it just falls back to transferring the file via q3 protocol.

CVARS client: 
cl_wwwDownload ... set to non-zero it tells the server the client would like redirects; this is attached to userinfo; this cvar depends on cl_allowDownload being enabled

CVARS server:
sv_wwwDownload ... depends on sv_allowDownload; non-zero is enabled; decides whether the server will send the client a redirect or not depending on whether the client has cl_wwwDownload set or not. 
sv_wwwBaseURL ... just like in ET, this is the base URL
sv_wwwDlDisconnected ... not implemented
sv_wwwFallbackURL ... not implemented
sv_wwwCheckPath ... not implemented

Like mentioned above, if either (client or server) doesn't support redirecting or either doesn't want to (xx_wwwDownload not set) they will default back to transfers via Q3 protocol.

To enable the above you have to set USE_CURL=1. The compilation depends on curl and pthreads (dynamically for Linux; pthreads-w32 statically for Mingw). No DLL needed on the client side. 

I know this is far from perfect. Especially the command interception in the client is ugly, but it seems to work on Linux and Windows. I did not try on Darwin, but it should work just like on Linux.

If you have any ideas on cleaning this up without breaking protocol 68 you're welcome to let me know your ideas.

(In reply to comment #4)
> If you have any ideas on cleaning this up without breaking protocol 68 you're
> welcome to let me know your ideas.

I've been working on this as well using cURL, but my version isn't working yet so you're way ahead :)

My version doesn't break the protocol because it doesn't add any new commands.  It only puts sv_wwwDownload and sv_wwwBaseURL into SERVERINFO.  If those values are adequate the client will use cURL instead of using UDP downloading.  Additionally, cl_wwwDownload is added to USERINFO so the server can disconnect the client if sv_wwwDlDisconnected is set.

Other variations are:
 * use of the curl_multi interface instead of requiring threads
 * optional dlopen() cURL loading (copied from the qal.c)

I will try to spend some time soon getting my version working before I can comment on the usefulness of these differences.

Created attachment 1038 [details]
svn873: curl support server changes

Simply adds sv_wwwDownload and sv_wwwBaseURL into the SYSTEMINFO string.

Created attachment 1039 [details]
svn873: curl support client changes

Adds two new files cl_curl.h and cl_curl.c which contain all the cURL specific code.

Created attachment 1040 [details]
unmodified curl 7.15.5 headers for use with LOCAL_HEADERS

These are useful for compiling with cURL support when cURL is not installed on the system.

I'm sure there are still a lot of bugs to work out with this.  I've done minimal testing on OSX and mingw.

By default, the mingw build tries to statically link to libcurl.  In order for this to work you need to have either libcurl.a or curl.lib in code/libs/win32/.  I haven't uploaded one of those since the libcurl.a built with mingw is enormous and  every curl.lib (MSVC) I've managed to create doesn't work with mingw.  I'm still working on this.

The linux and OSX builds use USE_CURL_DLOPEN as the default since most of those systems have a system libcurl.

I'm having second thoughts about adding sv_wwwDownload and sv_wwwBaseURL into SYSTEMINFO.  I'm thinking it might be worth the potential hazard and additional code to add them to SERVERINFO instead.

Pros for using SYSTEMINFO (currently using this):

1) there should be plenty of free space left in its 8192 bytes, sv_wwwBaseURL can be pretty long  (legitimately too).

2) server operators can't fill it up with cvars (no 'sets' command for adding cvars to it) so there is less of a chance of this causing problems with existing server configs.

3) SYSTEMINFO is already being parsed out prior to staring downloads.

Pros for using SERVERINFO:
1) unmodified servers could simply use the 'sets' command to add sv_wwwDownload and sv_wwwBaseURL into their SERVERINFO to enable these downloads.

2) people could see if the server supported these redirected downloads before connecting first (not sure if this is really useful).

Comments/Opinions?

(In reply to comment #10)
Sounds reasonable. 

Would sv_wwwDownload override sv_allowDownload? It does not do this in ET. There xx_wwwDownload depends on xx_allowDownload. I think this can be ignored though. Also I think that sv_wwwDlDisconnected should be implemented, as these cURL downloads are pretty fast. I say, let the server admins decide. O/c it would still be the client to behave accordingly.

Good idea, bad idea?

(In reply to comment #11)
> (In reply to comment #10)
> Sounds reasonable. 
> 
> Would sv_wwwDownload override sv_allowDownload? It does not do this in ET.
> There xx_wwwDownload depends on xx_allowDownload.

No, sv_allowDownload needs to be set to non-zero as well.

Likewise, cl_wwwDownload is only looked at if cl_allowDownload is non-zero.

> I think this can be ignored
> though. Also I think that sv_wwwDlDisconnected should be implemented, as these
> cURL downloads are pretty fast. I say, let the server admins decide. O/c it
> would still be the client to behave accordingly.

When the client starts a redirected download it disconnects itself.  There is
no server setting since there is no way for the server to know for sure if the client has really started the redirected download.   The only way to allow this to be a server setting would be to make a protocol change.  In my opinion, this is not necessary and they client should always disconnect when it starts a redirected download (the equiv of sv_wwwDlDisconnected 1).

(In reply to comment #12)
> > Also I think that sv_wwwDlDisconnected should be implemented, as these
> > cURL downloads are pretty fast. I say, let the server admins decide. O/c it
> > would still be the client to behave accordingly.
> 
> When the client starts a redirected download it disconnects itself.  There is
> no server setting since there is no way for the server to know for sure if the
> client has really started the redirected download.   The only way to allow this
> to be a server setting would be to make a protocol change.  In my opinion, this
> is not necessary and they client should always disconnect when it starts a
> redirected download (the equiv of sv_wwwDlDisconnected 1).
> 
Well, the server doesn't need to know in my opinion. It would be totally up to the client to play by the rules. This could also be 'sent' via SERVERINFO, defaulting to disconnect. 

(In reply to comment #13)
> Well, the server doesn't need to know in my opinion. It would be totally up to
> the client to play by the rules. This could also be 'sent' via SERVERINFO,
> defaulting to disconnect. 

The trouble is in the current implementation, if the client doesn't disconnect, then they will timeout anyway since they aren't communicating with the server while downloading.  Maybe there's a way to work around this, but it seems like it would be a whole lot simpler to just always disconnect.

(In reply to comment #14)
hmmm, my earlier patch did exactly this, called wwwDownload each frame.

Created attachment 1044 [details]
svn880: curl cupport client changes

* prevent a crash on glibc systems. CL_cURL_Cleanup() should not call curl_easy_cleanup() on a curl handle that has been added to a multi handle.

* should now build properly without curl support (USE_CURL=0)

Created attachment 1047 [details]
code/libs/win32/libcurl.a

static curl 7.15.5 lib for mingw

Configured with:

CFLAGS="-0s" ./configure --disable-crypto-auth --disable-cookies --disable-fi
le --disable-manual --disable-tftp --disable-telnet --disable-dict --disable-ss
pi --disable-debug --disable-ipv6 --disable-shared --disable-ares 

Thanks Hobbes.

Created attachment 1048 [details]
svn881 curl support


* switches to using SERVERINFO

* honors sv_allowDownload for cURL downloads

* stops trying to communicate with server while downloading with cURL

* attempts to download all missing files before reconnect (previously would reconnect for every file)

* URL isn't draw in UI anymore (looked bad in q3a ui).  It is still printed to console though

* URL added to download failure Com_Error() message

Created attachment 1049 [details]
svn882 curl support

* added sv_wwwDlDisconnected (default is 1) This is another SERVERINFO cvar :(

* added missing qcurl_easy_cleanup() call in CL_cURL_Cleanup() (thanks Hobbes)

* fixed cross compiling for mingw (thanks Hobbes)

Created attachment 1059 [details]
svn897 curl support

It's become clear that SERVERINFO string pollution is a real problem.  This is because this string provides an easy medium for communication between game and cgame code.  The result is that certain mods have used up nearly all of the available space in this string with cvars from the game code.  In order to keep as much compatability as possible I've adjusted this patch to make a much smaller footprint in SERVERINFO:

* sv_wwwDlDisconnected and sv_wwwDownload cvars have been replaced turning
  the existing sv_allowDownload cvar value into a bitflag-type setting:

  1 - allow udp downloads
  2 - allow redirected downloads (cURL)
  4 - disconnect on download (only applies to cURL downloads)

  This also gives the server admin added flexibility in that he/she can choose
  to allow ONLY cURL downloads.

* cl_wwwBaseURL renamed to cl_dlURL.  
  This is obviously a minor space savings, but there is really no sense in
  maintaining the ET cvar names at this point and besides "www" is a misnomer 
  since ftp can be used as well.

Created attachment 1061 [details]
svn897 curl support (2)


Drops the cvar cl_wwwDownload and replaces it with cl_downloadMethod:

cl_downloadMethod [integer]
     this is a bitmask cvar that supports the following flags:

     1 - allow udp downloads
     2 - allow download redirection (cURL)

     the valid values become:

     1 - only download with UDP
     2 - only download with cURL
     3 - first try to use cURL, but if it's not supported, fall back to UDP

     default is 3.

Note that it is possible to disable all downloading with this cvar by
effectively disabling all download methods, but cl_allowDownload must be set
to non-0 to actually ENABLE downloading.  The reason this new cvar was added
instead of making cl_allowDownload a bitmask value is that cl_allowDownload
MUST default to 0 for security reasons, but redirected download should be
enabled by default if downloading is turned on.

Created attachment 1067 [details]
svn897 curl support (3)

sorry for all the jumping around on this, but I'm changing cvars yet again.  I hope this is the last time :)

sv_dlURL
   (unchanged from last patch) string for the base of the download URL

sv_allowDownload
  bitmask value supports the following flags:

  1 - ENABLE (use curl if available, otherwise fall back to UDP if available,
      also ask clients to disconnect if using curl)
  2 - do not allow curl to be used
  4 - do not ask clients to disconnect when curl is used
  8 - do not allow UDP downloads to be used


cl_allowDownload
  bitmask value supports the following flags:

  1 - ENABLE (allow curl downloads if available, also allow UDP downloads)
  2 - do not allow curl downloads
  4 - do not allow UDP downloads

  this allows the patch to drop cl_downloadMethod while still having the
  new default behaviour if cl_allowDownload is 1.

cl_cURLLib
  the name of the curl dynamic library to be used when compiled with
  USE_CURL_DLOPEN=1 (the default)

Created attachment 1068 [details]
svn897 curl support (4)

oops, flags were supposed to be:

#define DLF_ENABLE 1
#define DLF_NO_REDIRECT 2
#define DLF_NO_UDP 4
#define DLF_NO_DISCONNECT 8

Created attachment 1069 [details]
svn897 curl README

Created attachment 1070 [details]
svn897 curl README (2)

checked in at revision 898

Closed for the win.

honestly using a external lib is really ugly, why not use a static version of libcurl and make it more like ET with the cvar sets, and really storing the url in either serverinfo/systeminfo are both bad choices, maybe make another type of cvar or something or another method to send to client because what if a server wishes to use an ftp link with a user/pass and does not want to give users direct access to it?

Setting a QA contact on all ioquake3 bugs, even resolved ones. Sorry if you get a flood of email from this, it should only happen once. Apologies for the incovenience.

--ryan.