skullernet / q2pro

Enhanced Quake 2 client and server
GNU General Public License v2.0
244 stars 82 forks source link

Fix HTTP downloads being started even when HTTP downloads should be aborted #348

Closed res2k closed 5 months ago

res2k commented 5 months ago

When a HTTP "fatal" error is encountered, such as a hostname resolution failure, HTTP downloads are supposed to be disabled entirely. However, despite that, HTTP downloads were still attempted.

The sequence of events is the following:

  1. HTTP_RunDownloads calls process_downloads
  2. In process_downloads, a DNS error is detected, causing fatal_error to be indicated, and abort_downloads called
  3. HTTP_CleanupDownloads() is called, which cleans up curl_multi
  4. However, after exiting process_downloads(), start_next_download() is called, which happily starts a new CURL download ... which is never checked for completion, as any subsequent HTTP_RunDownloads call will exit early.

The solution I opted for was to add check for curl_multi in start_next_download(). Another idea might be to check for curl_multi right before the start_next_download() call in HTTP_RunDownloads().

To reproduce the issue, find a server with a map or files you don't have (so a download is triggered) and provoke a DNS failure, eg by hacking in a bogus HTTP server name.

skullernet commented 5 months ago

Nice catch, thanks! Your fix looks good, applied.