From 68c3ea43b1e50f16ac92970e4e7b067f7cf6467d Mon Sep 17 00:00:00 2001 From: ac-stef Date: Mon, 7 Sep 2015 12:21:46 +0200 Subject: [PATCH] improved http error handling * don't treat 404 as http error * real errors return -1 and disconnect * -1 is treated as real error --- source/src/autodownload.cpp | 10 +++++----- source/src/http.cpp | 17 ++++++++++------- source/src/serverbrowser.cpp | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/source/src/autodownload.cpp b/source/src/autodownload.cpp index 59ed8a24..e47b8d8d 100644 --- a/source/src/autodownload.cpp +++ b/source/src/autodownload.cpp @@ -82,7 +82,7 @@ int pingpckserver(void *data) // fetch updates.txt from a media server, measure if(*u.port) h.set_port(atoi(u.port)); defformatstring(url)("%s%s", u.path, updatestxt); int res = h.get(url, 5000, 10000, 0, true); // http HEAD request only - to get a nice ping value - if(h.response > 0) + if(res >= 0 && h.response > 0) { s->ping = h.elapsedtime; SDL_mutexP(pckpinglog_lock); @@ -107,7 +107,7 @@ int pingpckserver(void *data) // fetch updates.txt from a media server, measure else { SDL_mutexP(pckpinglog_lock); - cvecprintf(pckping_log, "lretrieving %s:%d%s failed: response %d%s%s\n", u.domain, h.ip.port, url, h.response, h.err ? ", err: " : "", h.err ? h.err : ""); + cvecprintf(pckping_log, "lretrieving %s:%d%s failed: response %d, err: %s\n", u.domain, h.ip.port, url, h.response, h.err ? h.err : "(null)"); #ifdef _DEBUG if(DEBUGCOND) { @@ -123,7 +123,7 @@ int pingpckserver(void *data) // fetch updates.txt from a media server, measure else { // connection to server failed SDL_mutexP(pckpinglog_lock); - cvecprintf(pckping_log, "lpinging %s failed%s%s\n", s->host, h.err ? ", err: " : "", h.err ? h.err : ""); + cvecprintf(pckping_log, "lpinging %s failed, err: %s\n", s->host, h.err ? h.err : "(null)"); SDL_mutexV(pckpinglog_lock); } } @@ -384,10 +384,10 @@ bool dlpackage(httpget &h, package *pck, pckserver *s) // download one package f h.outstream = openvecfile(NULL); defformatstring(url)("%s%s", u.path, pck->requestname); int got = h.get(url, 6000, 90000); - if(!h.response) + if(got < 0 || !h.response) { if(!canceldownloads && s) s->ping = 0; // received a hard error from the server connection, better not try this one again - clientlogf("download %s:%d%s failed%s%s", u.domain, h.ip.port, url, h.err ? ", err: " : "", h.err ? h.err : ""); + clientlogf("download %s:%d%s failed, err: %s", u.domain, h.ip.port, url, h.err ? h.err : "(null)"); h.disconnect(); } else diff --git a/source/src/http.cpp b/source/src/http.cpp index 4f7a09e2..ff2ed85f 100644 --- a/source/src/http.cpp +++ b/source/src/http.cpp @@ -6,7 +6,7 @@ #define CR 13 #define LF 10 #define TCP_TIMEOUT 5678 // only re-use quite fresh connections -#define ERROR(msg) { err = msg; return -1; } +#define ERROR(msg) { err = msg; goto geterror; } const char lfcrlf[] = { LF, CR, LF, 0x00 }, *crlf = lfcrlf + 1; @@ -108,7 +108,7 @@ int httpget::get(const char *url1, uint timeout, uint totaltimeout, int range, b bool reconnected = false, retry = false; for(;;) { // (in a loop because of possible redirects) - if(!connect()) return -1; + if(!connect()) goto geterror; // assemble GET request cvecprintf(rawsnd, "%s %s HTTP/1.1%s", head ? "HEAD" : "GET", url, crlf); @@ -148,7 +148,7 @@ int httpget::get(const char *url1, uint timeout, uint totaltimeout, int range, b } elapsedtime = to.elapsed(); if(elapsedtime > totaltimeout || elapsedtime > lastresponse + timeout) ERROR("timeout 1"); - if(execcallback(-float(elapsedtime - lastresponse) / timeout)) return -1; // show progress bar, check for user interrupt + if(execcallback(-float(elapsedtime - lastresponse) / timeout)) goto geterror; // show progress bar, check for user interrupt } // get response @@ -197,8 +197,8 @@ int httpget::get(const char *url1, uint timeout, uint totaltimeout, int range, b if(p) p = strchr(p, ' '); response = p ? atoi(p + 1) : -1; if(p) p = strchr(p, '\n'); - if(!p || response < 200 || response > 307) ERROR("server error"); - if(response >= 300) // handle redirects + if(!p) ERROR("server error"); // header frame error + if(response >= 300 && response < 308) // handle redirects { p = strstr(header_uc + (p - header), "LOCATION: "); if(p) p = header + (p - header_uc); @@ -274,7 +274,7 @@ int httpget::get(const char *url1, uint timeout, uint totaltimeout, int range, b } rawrec.drop(); } - if(execcallback(progress)) return -1; // show progress bar, check for user interrupt + if(execcallback(progress)) goto geterror; // show progress bar, check for user interrupt if(content || (haveheader && head)) break; } if(retry && !reconnected) @@ -325,9 +325,12 @@ int httpget::get(const char *url1, uint timeout, uint totaltimeout, int range, b if(outvec) outvec->insert(outvec->length(), (const uchar *)content->getbuf(), res); if(outstream) outstream->write(content->getbuf(), res); } + +geterror: if(res > maxsize) err = "output truncated"; elapsedtime = to.elapsed(); - return res; + if(err) disconnect(); + return err ? -1 : res; } char *urlencode(const char *s, bool strict) diff --git a/source/src/serverbrowser.cpp b/source/src/serverbrowser.cpp index 99fa1484..4bcd9fd4 100644 --- a/source/src/serverbrowser.cpp +++ b/source/src/serverbrowser.cpp @@ -1330,8 +1330,8 @@ void retrieveservers(vector &data) defformatstring(url)("/retrieve.do?action=list&name=%s&version=%d&build=%d", urlencode(global_name, true), AC_VERSION, getbuildtype()|(1<<16)); h.outvec = (vector *) &data; // ouch... show_out_of_renderloop_progress(0, progresstext); - h.get(url, RETRIEVELIMIT, RETRIEVELIMIT); - if(h.response != 200) data.setsize(0); + int got = h.get(url, RETRIEVELIMIT, RETRIEVELIMIT); + if(got < 0 || h.response != 200) data.setsize(0); h.outvec = NULL; // must not be cleaned up by httpget if(data.length()) data.add('\0'); clfail = false;