From e8d4deecad3c0b2defb3059ed11eaae24feb2e81 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 3 Nov 2011 00:31:27 +0900 Subject: [PATCH] Rewritten HttpHeader::fill() --- src/HttpHeader.cc | 131 ++++++++++++++++++++------------ src/HttpHeader.h | 5 +- src/HttpHeaderProcessor.cc | 17 +++-- src/HttpHeaderProcessor.h | 4 +- test/HttpHeaderProcessorTest.cc | 15 +++- test/HttpHeaderTest.cc | 71 ++++++++++++++--- 6 files changed, 171 insertions(+), 72 deletions(-) diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc index f3b90eb0..a3e53def 100644 --- a/src/HttpHeader.cc +++ b/src/HttpHeader.cc @@ -33,9 +33,6 @@ */ /* copyright --> */ #include "HttpHeader.h" - -#include - #include "Range.h" #include "util.h" #include "A2STR.h" @@ -152,38 +149,37 @@ RangeHandle HttpHeader::getRange() const } } } - std::string byteRangeSpec; - { - // we expect that rangeStr looks like 'bytes 100-199/100' - // but some server returns '100-199/100', omitting bytes-unit sepcifier - // 'bytes'. - std::pair splist; - util::divide(splist, rangeStr, ' '); - if(splist.second.empty()) { - // we assume bytes-unit specifier omitted. - byteRangeSpec = splist.first; - } else { - byteRangeSpec = splist.second; + // we expect that rangeStr looks like 'bytes 100-199/100' + // but some server returns '100-199/100', omitting bytes-unit sepcifier + // 'bytes'. + std::string::const_iterator byteRangeSpec = + std::find(rangeStr.begin(), rangeStr.end(), ' '); + if(byteRangeSpec == rangeStr.end()) { + // we assume bytes-unit specifier omitted. + byteRangeSpec = rangeStr.begin(); + } else { + while(byteRangeSpec != rangeStr.end() && + (*byteRangeSpec == ' ' || *byteRangeSpec == '\t')) { + ++byteRangeSpec; } } - std::pair byteRangeSpecPair; - util::divide(byteRangeSpecPair, byteRangeSpec, '/'); - - if(util::strip(byteRangeSpecPair.first) == "*" || - util::strip(byteRangeSpecPair.second) == "*") { + std::string::const_iterator slash = + std::find(byteRangeSpec, rangeStr.end(), '/'); + if(slash == rangeStr.end() || slash+1 == rangeStr.end() || + (byteRangeSpec+1 == slash && *byteRangeSpec == '*') || + (slash+2 == rangeStr.end() && *(slash+1) == '*')) { // If byte-range-resp-spec or instance-length is "*", we returns // empty Range. The former is usually sent with 416 (Request range // not satisfiable) status. return SharedHandle(new Range()); } - - std::pair byteRangeRespSpecPair; - util::divide(byteRangeRespSpecPair, byteRangeSpecPair.first, '-'); - - off_t startByte = util::parseLLInt(byteRangeRespSpecPair.first); - off_t endByte = util::parseLLInt(byteRangeRespSpecPair.second); - uint64_t entityLength = util::parseULLInt(byteRangeSpecPair.second); - + std::string::const_iterator minus = std::find(byteRangeSpec, slash, '-'); + if(minus == slash) { + return SharedHandle(new Range()); + } + off_t startByte = util::parseLLInt(std::string(byteRangeSpec, minus)); + off_t endByte = util::parseLLInt(std::string(minus+1, slash)); + uint64_t entityLength = util::parseULLInt(std::string(slash+1, rangeStr.end())); return SharedHandle(new Range(startByte, endByte, entityLength)); } @@ -202,28 +198,67 @@ void HttpHeader::setRequestPath(const std::string& requestPath) requestPath_ = requestPath; } -void HttpHeader::fill(std::istream& in) +namespace { +template +std::pair stripIter2 +(InputIterator first, InputIterator last, + const std::string& chars = "\r\n \t") { - std::string line; - std::getline(in, line); - while(in) { - line = util::strip(line); - if(line.empty()) { - std::getline(in, line); - } else { - std::pair hp; - util::divide(hp, line, ':'); - while(std::getline(in, line)) { - if(!line.empty() && (line[0] == ' ' || line[0] == '\t')) { - line = util::strip(line); - hp.second += " "; - hp.second += line; - } else { - break; - } - } - put(hp.first, hp.second); + for(; first != last && + std::find(chars.begin(), chars.end(), *first) != chars.end(); ++first); + if(first == last) { + return std::make_pair(first, last); + } + InputIterator left = last-1; + for(; left != first && + std::find(chars.begin(), chars.end(), *left) != chars.end(); --left); + return std::make_pair(first, left+1); +} +} // namespace + +void HttpHeader::fill +(std::string::const_iterator first, + std::string::const_iterator last) +{ + std::string name; + std::string value; + while(first != last) { + std::string::const_iterator j = first; + while(j != last && *j != '\r' && *j != '\n') { + ++j; } + if(first != j) { + std::string::const_iterator sep = std::find(first, j, ':'); + if(sep == j) { + // multiline header? + if(*first == ' ' || *first == '\t') { + std::pair p = stripIter2(first, j); + if(!name.empty() && p.first != p.second) { + if(!value.empty()) { + value += ' '; + } + value.append(p.first, p.second); + } + } + } else { + if(!name.empty()) { + put(name, value); + } + std::pair p = stripIter2(first, sep); + name.assign(p.first, p.second); + p = stripIter2(sep+1, j); + value.assign(p.first, p.second); + } + } + while(j != last && (*j == '\r' || *j == '\n')) { + ++j; + } + first = j; + } + if(!name.empty()) { + put(name, value); } } diff --git a/src/HttpHeader.h b/src/HttpHeader.h index df2e9216..a9be6660 100644 --- a/src/HttpHeader.h +++ b/src/HttpHeader.h @@ -40,7 +40,6 @@ #include #include #include -#include #include "SharedHandle.h" @@ -110,7 +109,9 @@ public: void setRequestPath(const std::string& requestPath); - void fill(std::istream& in); + void fill + (std::string::const_iterator first, + std::string::const_iterator last); // Clears table_. responseStatus_ and version_ are unchanged. void clearField(); diff --git a/src/HttpHeaderProcessor.cc b/src/HttpHeaderProcessor.cc index 477d7458..978711dc 100644 --- a/src/HttpHeaderProcessor.cc +++ b/src/HttpHeaderProcessor.cc @@ -34,7 +34,6 @@ /* copyright --> */ #include "HttpHeaderProcessor.h" -#include #include #include "HttpHeader.h" @@ -60,7 +59,7 @@ HttpHeaderProcessor::~HttpHeaderProcessor() {} void HttpHeaderProcessor::update(const unsigned char* data, size_t length) { checkHeaderLimit(length); - buf_ += std::string(&data[0], &data[length]); + buf_.append(&data[0], &data[length]); } void HttpHeaderProcessor::update(const std::string& data) @@ -119,10 +118,13 @@ SharedHandle HttpHeaderProcessor::getHttpResponseHeader() HttpHeaderHandle httpHeader(new HttpHeader()); httpHeader->setVersion(buf_.substr(0, 8)); httpHeader->setStatusCode(statusCode); - std::istringstream strm(buf_); // TODO 1st line(HTTP/1.1 200...) is also send to HttpHeader, but it should // not. - httpHeader->fill(strm); + if((delimpos = buf_.find("\r\n\r\n")) == std::string::npos && + (delimpos = buf_.find("\n\n")) == std::string::npos) { + delimpos = buf_.size(); + } + httpHeader->fill(buf_.begin(), buf_.begin()+delimpos); return httpHeader; } @@ -147,8 +149,11 @@ SharedHandle HttpHeaderProcessor::getHttpRequestHeader() httpHeader->setMethod(firstLine[0]); httpHeader->setRequestPath(firstLine[1]); httpHeader->setVersion(firstLine[2]); - std::istringstream strm(buf_.substr(delimpos)); - httpHeader->fill(strm); + if((delimpos = buf_.find("\r\n\r\n")) == std::string::npos && + (delimpos = buf_.find("\n\n")) == std::string::npos) { + delimpos = buf_.size(); + } + httpHeader->fill(buf_.begin(), buf_.begin()+delimpos); return httpHeader; } diff --git a/src/HttpHeaderProcessor.h b/src/HttpHeaderProcessor.h index b225b31d..b352108f 100644 --- a/src/HttpHeaderProcessor.h +++ b/src/HttpHeaderProcessor.h @@ -36,10 +36,12 @@ #define D_HTTP_HEADER_PROCESSOR_H #include "common.h" -#include "SharedHandle.h" + #include #include +#include "SharedHandle.h" + namespace aria2 { class HttpHeader; diff --git a/test/HttpHeaderProcessorTest.cc b/test/HttpHeaderProcessorTest.cc index e287538c..33813cf6 100644 --- a/test/HttpHeaderProcessorTest.cc +++ b/test/HttpHeaderProcessorTest.cc @@ -1,9 +1,12 @@ #include "HttpHeaderProcessor.h" + +#include + +#include + #include "HttpHeader.h" #include "DlRetryEx.h" #include "DlAbortEx.h" -#include -#include namespace aria2 { @@ -102,7 +105,8 @@ void HttpHeaderProcessorTest::testGetHttpResponseHeader() "Content-Length: 9187\r\n" "Connection: close\r\n" "Content-Type: text/html; charset=UTF-8\r\n" - "\r\n"; + "\r\n" + "Entity: body"; proc.update(hd); @@ -116,6 +120,7 @@ void HttpHeaderProcessorTest::testGetHttpResponseHeader() CPPUNIT_ASSERT_EQUAL((uint64_t)9187ULL, header->getFirstAsULLInt("Content-Length")); CPPUNIT_ASSERT_EQUAL(std::string("text/html; charset=UTF-8"), header->getFirst("Content-Type")); + CPPUNIT_ASSERT(!header->defined("entity")); } void HttpHeaderProcessorTest::testGetHttpResponseHeader_empty() @@ -208,7 +213,8 @@ void HttpHeaderProcessorTest::testGetHttpRequestHeader() std::string request = "GET /index.html HTTP/1.1\r\n" "Host: host\r\n" "Connection: close\r\n" - "\r\n"; + "\r\n" + "Entity: body"; proc.update(request); @@ -218,6 +224,7 @@ void HttpHeaderProcessorTest::testGetHttpRequestHeader() CPPUNIT_ASSERT_EQUAL(std::string("/index.html"),httpHeader->getRequestPath()); CPPUNIT_ASSERT_EQUAL(std::string("HTTP/1.1"), httpHeader->getVersion()); CPPUNIT_ASSERT_EQUAL(std::string("close"),httpHeader->getFirst("Connection")); + CPPUNIT_ASSERT(!httpHeader->defined("entity")); } } // namespace aria2 diff --git a/test/HttpHeaderTest.cc b/test/HttpHeaderTest.cc index baef6d00..e2dbed13 100644 --- a/test/HttpHeaderTest.cc +++ b/test/HttpHeaderTest.cc @@ -1,10 +1,9 @@ #include "HttpHeader.h" -#include - #include #include "Range.h" +#include "DlAbortEx.h" namespace aria2 { @@ -71,6 +70,56 @@ void HttpHeaderTest::testGetRange() CPPUNIT_ASSERT_EQUAL((off_t)0, range->getEndByte()); CPPUNIT_ASSERT_EQUAL((uint64_t)0, range->getEntityLength()); } + { + HttpHeader httpHeader; + httpHeader.put("Content-Range", "bytes */*"); + + SharedHandle range = httpHeader.getRange(); + + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getStartByte()); + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getEndByte()); + CPPUNIT_ASSERT_EQUAL((uint64_t)0, range->getEntityLength()); + } + { + HttpHeader httpHeader; + httpHeader.put("Content-Range", "bytes 0"); + + SharedHandle range = httpHeader.getRange(); + + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getStartByte()); + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getEndByte()); + CPPUNIT_ASSERT_EQUAL((uint64_t)0, range->getEntityLength()); + } + { + HttpHeader httpHeader; + httpHeader.put("Content-Range", "bytes 0/"); + + SharedHandle range = httpHeader.getRange(); + + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getStartByte()); + CPPUNIT_ASSERT_EQUAL((off_t)0, range->getEndByte()); + CPPUNIT_ASSERT_EQUAL((uint64_t)0, range->getEntityLength()); + } + { + HttpHeader httpHeader; + httpHeader.put("Content-Range", "bytes 0-/3"); + try { + httpHeader.getRange(); + CPPUNIT_FAIL("Exception must be thrown"); + } catch(const DlAbortEx& e) { + // success + } + } + { + HttpHeader httpHeader; + httpHeader.put("Content-Range", "bytes -0/3"); + try { + httpHeader.getRange(); + CPPUNIT_FAIL("Exception must be thrown"); + } catch(const DlAbortEx& e) { + // success + } + } } void HttpHeaderTest::testGet() @@ -104,16 +153,16 @@ void HttpHeaderTest::testClearField() void HttpHeaderTest::testFill() { - std::stringstream ss; - ss << "Host: aria2.sourceforge.net\r\n" - << "Connection: close \r\n" // trailing white space - << "Multi-Line: text1\r\n" - << " text2\r\n" - << " text3\r\n" - << "Duplicate: foo\r\n" - << "Duplicate: bar\r\n"; + std::string s = + "Host: aria2.sourceforge.net\r\n" + "Connection: close \r\n" // trailing white space + "Multi-Line: text1\r\n" + " text2\r\n" + " text3\r\n" + "Duplicate: foo\r\n" + "Duplicate: bar\r\n"; HttpHeader h; - h.fill(ss); + h.fill(s.begin(), s.end()); CPPUNIT_ASSERT_EQUAL(std::string("aria2.sourceforge.net"), h.getFirst("Host")); CPPUNIT_ASSERT_EQUAL(std::string("close"),