diff --git a/ChangeLog b/ChangeLog index 74dba579..2daef105 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2007-11-22 Tatsuhiro Tsujikawa + + Replaced strtol with Util::parseInt + * src/ChunkedEncoding.cc + * src/HttpConnection.cc + * src/CookieBoxFactory.cc + * src/ParameterizedStringParser.cc + * src/Util.cc + * test/UtilTest.cc + * test/OptionHandlerTest.cc + * src/Request.cc + + Throw exception when empty string is given. The message for exception + changed. + * src/Util.cc (parseInt)(parseLLInt) + * src/message.h + 2007-11-22 Tatsuhiro Tsujikawa Set precision to 2 because share ratio is rounded into 1.0 if precision diff --git a/TODO b/TODO index 1430eb33..d29e7038 100644 --- a/TODO +++ b/TODO @@ -53,7 +53,7 @@ DlAbortEx .... Abort download with the connection/url. Should be renamed to PermanentFailureException DownloadFailureException .... RequestGroup should halt. FatalException .... Program should abort. +* replace strtol with Util::parseInt -- remaining issues to be implemented for 0.12.0 release * Update translation -* replace strtol with Util::parseInt diff --git a/src/ChunkedEncoding.cc b/src/ChunkedEncoding.cc index b66439a4..c376aba1 100644 --- a/src/ChunkedEncoding.cc +++ b/src/ChunkedEncoding.cc @@ -35,6 +35,7 @@ #include "ChunkedEncoding.h" #include "DlAbortEx.h" #include "message.h" +#include "Util.h" #include #include #include @@ -155,23 +156,15 @@ int32_t ChunkedEncoding::readChunkSize(char** pp) { return -1; } p = rp; - // We ignore chunk-extension char* exsp = (char*)memchr(*pp, ';', strbufTail-*pp); - if(exsp == NULL) { + if(exsp == 0 || p < exsp) { exsp = p; } - // TODO check invalid characters in buffer - char* temp = new char[exsp-*pp+1]; - memcpy(temp, *pp, exsp-*pp); - temp[exsp-*pp] = '\0'; - - chunkSize = strtol(temp, NULL, 16); - delete [] temp; + string temp(*pp, exsp); + chunkSize = Util::parseInt(temp, 16); if(chunkSize < 0) { throw new DlAbortEx(EX_INVALID_CHUNK_SIZE); - } else if(errno == ERANGE && (chunkSize == INT32_MAX || chunkSize == INT32_MIN)) { - throw new DlAbortEx(strerror(errno)); } *pp = p+2; return 0; diff --git a/src/CookieBoxFactory.cc b/src/CookieBoxFactory.cc index 1d231510..2020d5b4 100644 --- a/src/CookieBoxFactory.cc +++ b/src/CookieBoxFactory.cc @@ -68,7 +68,7 @@ Cookie CookieBoxFactory::parseNsCookie(const string& nsCookieStr) const c.domain = vs[0]; c.path = vs[2]; c.secure = vs[3] == "TRUE" ? true : false; - c.expires = strtol(vs[4].c_str(), NULL, 10); + c.expires = Util::parseInt(vs[4]); c.name = vs[5]; if(vs.size() >= 7) { c.value = vs[6]; diff --git a/src/HttpConnection.cc b/src/HttpConnection.cc index 57ebbbaf..072e9f34 100644 --- a/src/HttpConnection.cc +++ b/src/HttpConnection.cc @@ -115,7 +115,7 @@ HttpResponseHandle HttpConnection::receiveResponse() HttpResponseHandle httpResponse = new HttpResponse(); httpResponse->setCuid(cuid); - httpResponse->setStatus(strtol(httpStatusHeader.first.c_str(), 0, 10)); + httpResponse->setStatus(Util::parseInt(httpStatusHeader.first)); httpResponse->setHttpHeader(httpStatusHeader.second); httpResponse->setHttpRequest(entry->getHttpRequest()); diff --git a/src/ParameterizedStringParser.cc b/src/ParameterizedStringParser.cc index 4eef3574..409f5847 100644 --- a/src/ParameterizedStringParser.cc +++ b/src/ParameterizedStringParser.cc @@ -111,7 +111,7 @@ PStringDatumHandle ParameterizedStringParser::createLoop(const string& src, if(colonIndex != string::npos) { string stepStr = loopStr.substr(colonIndex+1); if(Util::isNumber(stepStr)) { - step = strtol(stepStr.c_str(), 0, 10); + step = Util::parseInt(stepStr); } else { throw new FatalException("A step count must be a number."); } @@ -126,8 +126,8 @@ PStringDatumHandle ParameterizedStringParser::createLoop(const string& src, int32_t end; if(Util::isNumber(range.first) && Util::isNumber(range.second)) { nd = new FixedWidthNumberDecorator(range.first.size()); - start = strtol(range.first.c_str(), 0, 10); - end = strtol(range.second.c_str(), 0, 10); + start = Util::parseInt(range.first); + end = Util::parseInt(range.second); } else if(Util::isLowercase(range.first) && Util::isLowercase(range.second)) { nd = new AlphaNumberDecorator(range.first.size()); start = Util::alphaToNum(range.first); diff --git a/src/Request.cc b/src/Request.cc index 69cf025e..c913ea90 100644 --- a/src/Request.cc +++ b/src/Request.cc @@ -36,6 +36,7 @@ #include "Util.h" #include "FeatureConfig.h" #include "CookieBoxFactory.h" +#include "RecoverableException.h" const string Request::METHOD_GET = "get"; @@ -113,8 +114,13 @@ bool Request::parseUrl(const string& url) { Util::split(hostAndPort, hostPart, ':'); host = hostAndPort.first; if(hostAndPort.second != "") { - port = strtol(hostAndPort.second.c_str(), NULL, 10); - if(!(0 < port && port <= 65535)) { + try { + port = Util::parseInt(hostAndPort.second); + if(!(0 < port && port <= 65535)) { + return false; + } + } catch(RecoverableException* e) { + delete e; return false; } } else { diff --git a/src/Util.cc b/src/Util.cc index ee93b4d9..d11a35e8 100644 --- a/src/Util.cc +++ b/src/Util.cc @@ -304,11 +304,7 @@ string Util::urldecode(const string& target) { if(*itr == '%') { if(itr+1 != target.end() && itr+2 != target.end() && isxdigit(*(itr+1)) && isxdigit(*(itr+2))) { - char temp[3]; - temp[0] = *(itr+1); - temp[1] = *(itr+2); - temp[2] = '\0'; - result += strtol(temp, 0, 16); + result += Util::parseInt(string(itr+1, itr+3), 16); itr += 2; } else { result += *itr; @@ -466,26 +462,38 @@ void Util::unfoldRange(const string& src, Integers& range) { int32_t Util::parseInt(const string& s, int32_t base) { + if(s.empty()) { + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "empty string"); + } char* stop; errno = 0; long int v = strtol(s.c_str(), &stop, base); if(*stop != '\0') { - throw new DlAbortEx(MSG_ILLEGAL_CHARACTER, stop); + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "illegal character"); } else if((v == LONG_MIN || v == LONG_MAX) && errno == ERANGE || v > INT32_MAX || v < INT32_MIN) { - throw new DlAbortEx(MSG_OVERFLOW_UNDERFLOW_DETECTED, s.c_str()); + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "overflow/underflow"); } return v; } int64_t Util::parseLLInt(const string& s, int32_t base) { + if(s.empty()) { + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "empty string"); + } char* stop; errno = 0; int64_t v = strtoll(s.c_str(), &stop, base); if(*stop != '\0') { - throw new DlAbortEx(MSG_ILLEGAL_CHARACTER, stop); + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "illegal character"); } else if((v == INT64_MIN || v == INT64_MAX) && errno == ERANGE) { - throw new DlAbortEx(MSG_OVERFLOW_UNDERFLOW_DETECTED, s.c_str()); + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "overflow/underflow"); } return v; } @@ -646,7 +654,15 @@ int64_t Util::getRealSize(const string& sizeWithUnit) } size = sizeWithUnit.substr(0, p); } - return strtoll(size.c_str(), 0, 10)*mult; + int64_t v = Util::parseLLInt(size); + + if(v < 0) { + throw new DlAbortEx("Negative value detected: %s", sizeWithUnit.c_str()); + } else if(v*mult < 0) { + throw new DlAbortEx(MSG_STRING_INTEGER_CONVERSION_FAILURE, + "overflow/underflow"); + } + return v*mult; } string Util::abbrevSize(int64_t size) diff --git a/src/message.h b/src/message.h index 3a2f82d2..b8fb0b50 100644 --- a/src/message.h +++ b/src/message.h @@ -126,9 +126,8 @@ #define MSG_DAEMON_FAILED _("daemon failed.") #define MSG_VERIFICATION_SUCCESSFUL _("Verification finished successfully. file=%s") #define MSG_VERIFICATION_FAILED _("Checksum error detected. file=%s") -#define MSG_ILLEGAL_CHARACTER _("Illegal character detected: %s") #define MSG_INCOMPLETE_RANGE _("Incomplete range specified. %s") -#define MSG_OVERFLOW_UNDERFLOW_DETECTED _("Overflow/underflow detected: %s") +#define MSG_STRING_INTEGER_CONVERSION_FAILURE _("Failed to convert string into value: %s") #define EX_TIME_OUT _("Timeout.") #define EX_INVALID_CHUNK_SIZE _("Invalid chunk size.") diff --git a/test/OptionHandlerTest.cc b/test/OptionHandlerTest.cc index c7486e6a..741f881e 100644 --- a/test/OptionHandlerTest.cc +++ b/test/OptionHandlerTest.cc @@ -151,12 +151,26 @@ void OptionHandlerTest::testUnitNumberOptionHandler() CPPUNIT_ASSERT_EQUAL(string("4294967296"), option.get("foo")); handler.parse(&option, "4096K"); CPPUNIT_ASSERT_EQUAL(string("4194304"), option.get("foo")); - handler.parse(&option, "K"); - CPPUNIT_ASSERT_EQUAL(string("0"), option.get("foo")); - handler.parse(&option, "M"); - CPPUNIT_ASSERT_EQUAL(string("0"), option.get("foo")); - handler.parse(&option, ""); - CPPUNIT_ASSERT_EQUAL(string("0"), option.get("foo")); + try { + handler.parse(&option, "K"); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + handler.parse(&option, "M"); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + handler.parse(&option, ""); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } } void OptionHandlerTest::testParameterOptionHandler_1argInit() diff --git a/test/UtilTest.cc b/test/UtilTest.cc index 183b72fb..188fad7f 100644 --- a/test/UtilTest.cc +++ b/test/UtilTest.cc @@ -305,8 +305,41 @@ void UtilTest::testGetRealSize() { CPPUNIT_ASSERT_EQUAL((int64_t)4294967296LL, Util::getRealSize("4096M")); CPPUNIT_ASSERT_EQUAL((int64_t)1024, Util::getRealSize("1K")); - CPPUNIT_ASSERT_EQUAL((int64_t)0, Util::getRealSize("")); - CPPUNIT_ASSERT_EQUAL((int64_t)0, Util::getRealSize("foo")); + try { + Util::getRealSize(""); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + Util::getRealSize("foo"); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + Util::getRealSize("-1"); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + Util::getRealSize("9223372036854775807K"); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } + try { + Util::getRealSize("9223372036854775807M"); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } } void UtilTest::testAbbrevSize() @@ -504,7 +537,13 @@ void UtilTest::testParseInt() cerr << *e; delete e; } - + try { + Util::parseInt(""); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } } void UtilTest::testParseLLInt() @@ -533,5 +572,11 @@ void UtilTest::testParseLLInt() cerr << *e; delete e; } - + try { + Util::parseLLInt(""); + CPPUNIT_FAIL("exception must be thrown."); + } catch(Exception* e) { + cerr << *e; + delete e; + } }