diff --git a/ChangeLog b/ChangeLog index 3c1d4418..81d3221b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2008-07-19 Tatsuhiro Tsujikawa + + Supported absolute/relative path in Location header field. + * src/AbstractCommand.cc: Call resetUrl() when DlRetryEx is caught. + * src/HttpHeader.cc + * src/HttpHeader.h + * src/HttpResponse.cc + * src/HttpSkipResponseCommand.cc + * src/Request.cc + * test/HttpHeaderTest.cc + * test/HttpResponseTest.cc + * test/RequestTest.cc + 2008-07-16 Tatsuhiro Tsujikawa Changed help text for --load-cookie option. diff --git a/src/AbstractCommand.cc b/src/AbstractCommand.cc index 76773ee0..eb2f2aee 100644 --- a/src/AbstractCommand.cc +++ b/src/AbstractCommand.cc @@ -156,6 +156,8 @@ bool AbstractCommand::execute() { if(isAbort) { onAbort(); } + // In case where Request::getCurrentUrl() is not a valid URI. + req->resetUrl(); if(isAbort) { logger->info(MSG_MAX_TRY, cuid, req->getTryCount()); logger->error(MSG_DOWNLOAD_ABORTED, err, cuid, req->getUrl().c_str()); diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc index 2fcfed9e..6fea1009 100644 --- a/src/HttpHeader.cc +++ b/src/HttpHeader.cc @@ -205,4 +205,9 @@ void HttpHeader::fill(std::istream& in) } } +void HttpHeader::clearField() +{ + table.clear(); +} + } // namespace aria2 diff --git a/src/HttpHeader.h b/src/HttpHeader.h index 286e6eb7..4bbec4c4 100644 --- a/src/HttpHeader.h +++ b/src/HttpHeader.h @@ -79,6 +79,9 @@ public: void fill(std::istream& in); + // Clears table. _responseStatus and _version are unchanged. + void clearField(); + static const std::string LOCATION; static const std::string TRANSFER_ENCODING; diff --git a/src/HttpResponse.cc b/src/HttpResponse.cc index 86a929b2..db6c4a9a 100644 --- a/src/HttpResponse.cc +++ b/src/HttpResponse.cc @@ -44,6 +44,7 @@ #include "Util.h" #include "message.h" #include "DlAbortEx.h" +#include "DlRetryEx.h" #include "StringFormat.h" #include "A2STR.h" #include "Decoder.h" @@ -123,8 +124,16 @@ bool HttpResponse::isRedirect() const void HttpResponse::processRedirect() { - httpRequest->getRequest()->redirectUrl(getRedirectURI()); - + + if(httpRequest->getRequest()->redirectUrl(getRedirectURI())) { + logger->info(MSG_REDIRECT, cuid, + httpRequest->getRequest()->getCurrentUrl().c_str()); + } else { + throw DlRetryEx + (StringFormat("CUID#%d - Redirect to %s failed. It may not be a valid" + " URI.", cuid, + httpRequest->getRequest()->getCurrentUrl().c_str()).str()); + } } std::string HttpResponse::getRedirectURI() const diff --git a/src/HttpSkipResponseCommand.cc b/src/HttpSkipResponseCommand.cc index 0ed640cd..e89e0273 100644 --- a/src/HttpSkipResponseCommand.cc +++ b/src/HttpSkipResponseCommand.cc @@ -134,7 +134,6 @@ bool HttpSkipResponseCommand::processResponse() throw DlAbortEx(StringFormat("Too many redirects: count=%u", rnum).str()); } _httpResponse->processRedirect(); - logger->info(MSG_REDIRECT, cuid, _httpResponse->getRedirectURI().c_str()); return prepareForRetry(0); } else if(_httpResponse->hasRetryAfter()) { return prepareForRetry(_httpResponse->getRetryAfter()); diff --git a/src/Request.cc b/src/Request.cc index 0556b953..9a8a1530 100644 --- a/src/Request.cc +++ b/src/Request.cc @@ -80,7 +80,19 @@ bool Request::redirectUrl(const std::string& url) { previousUrl = A2STR::NIL; _supportsPersistentConnection = true; ++_redirectCount; - return parseUrl(url); + if(url.find("://") == std::string::npos) { + // rfc2616 requires absolute URI should be provided by Location header + // field, but some servers don't obey this rule. + if(Util::startsWith(url, "/")) { + // abosulute path + return parseUrl(protocol+"://"+host+url); + } else { + // relative path + return parseUrl(protocol+"://"+host+dir+"/"+url); + } + } else { + return parseUrl(url); + } } bool Request::parseUrl(const std::string& url) { diff --git a/test/HttpHeaderTest.cc b/test/HttpHeaderTest.cc index c377e54c..a0724b5c 100644 --- a/test/HttpHeaderTest.cc +++ b/test/HttpHeaderTest.cc @@ -9,11 +9,13 @@ class HttpHeaderTest:public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(HttpHeaderTest); CPPUNIT_TEST(testGetRange); CPPUNIT_TEST(testGet); + CPPUNIT_TEST(testClearField); CPPUNIT_TEST_SUITE_END(); public: void testGetRange(); void testGet(); + void testClearField(); }; @@ -58,4 +60,20 @@ void HttpHeaderTest::testGet() CPPUNIT_ASSERT_EQUAL(std::string("101"), r[1]); } +void HttpHeaderTest::testClearField() +{ + HttpHeader h; + h.setResponseStatus(HttpHeader::S200); + h.setVersion(HttpHeader::HTTP_1_1); + h.put("Foo", "Bar"); + + CPPUNIT_ASSERT_EQUAL(std::string("Bar"), h.getFirst("Foo")); + + h.clearField(); + + CPPUNIT_ASSERT_EQUAL(std::string(""), h.getFirst("Foo")); + CPPUNIT_ASSERT_EQUAL(HttpHeader::S200, h.getResponseStatus()); + CPPUNIT_ASSERT_EQUAL(HttpHeader::HTTP_1_1, h.getVersion()); +} + } // namespace aria2 diff --git a/test/HttpResponseTest.cc b/test/HttpResponseTest.cc index ce75fe95..d539b204 100644 --- a/test/HttpResponseTest.cc +++ b/test/HttpResponseTest.cc @@ -8,6 +8,7 @@ #include "Exception.h" #include "A2STR.h" #include "Decoder.h" +#include "DlRetryEx.h" #include #include @@ -38,6 +39,7 @@ class HttpResponseTest : public CppUnit::TestFixture { CPPUNIT_TEST(testValidateResponse_bad_range); CPPUNIT_TEST(testValidateResponse_chunked); CPPUNIT_TEST(testHasRetryAfter); + CPPUNIT_TEST(testProcessRedirect); CPPUNIT_TEST_SUITE_END(); private: @@ -66,6 +68,7 @@ public: void testValidateResponse_bad_range(); void testValidateResponse_chunked(); void testHasRetryAfter(); + void testProcessRedirect(); }; @@ -422,4 +425,33 @@ void HttpResponseTest::testHasRetryAfter() CPPUNIT_ASSERT_EQUAL((time_t)60, httpResponse.getRetryAfter()); } +void HttpResponseTest::testProcessRedirect() +{ + HttpResponse httpResponse; + SharedHandle httpHeader(new HttpHeader()); + httpResponse.setHttpHeader(httpHeader); + + SharedHandle httpRequest(new HttpRequest()); + SharedHandle request(new Request()); + request->setUrl("http://localhost/archives/aria2-1.0.0.tar.bz2"); + httpRequest->setRequest(request); + httpResponse.setHttpRequest(httpRequest); + + httpHeader->put("Location", "http://mirror/aria2-1.0.0.tar.bz2"); + httpResponse.processRedirect(); + + httpHeader->clearField(); + + // Give unsupported scheme + httpHeader->put("Location", "unsupported://mirror/aria2-1.0.0.tar.bz2"); + try { + httpResponse.processRedirect(); + CPPUNIT_FAIL("DlRetryEx exception must be thrown."); + } catch(DlRetryEx& e) { + // Success + } catch(...) { + CPPUNIT_FAIL("DlRetryEx exception must be thrown."); + } +} + } // namespace aria2 diff --git a/test/RequestTest.cc b/test/RequestTest.cc index 510b0f2c..807ec821 100644 --- a/test/RequestTest.cc +++ b/test/RequestTest.cc @@ -279,10 +279,12 @@ void RequestTest::testRedirectUrl() { // persistent connection flag is set to be true after redirection CPPUNIT_ASSERT(req.supportsPersistentConnection()); // url must be the same - CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.com:8080/aria2/index.html"), + CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.com:8080/aria2/" + "index.html"), req.getUrl()); // currentUrl must be updated - CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.co.jp/"), req.getCurrentUrl()); + CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.co.jp/"), + req.getCurrentUrl()); // previousUrl must be "" when redirection CPPUNIT_ASSERT_EQUAL(std::string(""), req.getPreviousUrl()); CPPUNIT_ASSERT_EQUAL(std::string("http"), req.getProtocol()); @@ -293,6 +295,19 @@ void RequestTest::testRedirectUrl() { CPPUNIT_ASSERT_EQUAL(std::string(""), req.getQuery()); // See redirect count is incremented. CPPUNIT_ASSERT_EQUAL((unsigned int)1, req.getRedirectCount()); + + // Give abosulute path + CPPUNIT_ASSERT(req.redirectUrl("/abspath/to/file")); + CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.co.jp/abspath/to/file"), + req.getCurrentUrl()); + CPPUNIT_ASSERT_EQUAL((unsigned int)2, req.getRedirectCount()); + + // Give relative path + CPPUNIT_ASSERT(req.redirectUrl("relativepath/to/file")); + CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.co.jp/abspath/to/" + "relativepath/to/file"), + req.getCurrentUrl()); + CPPUNIT_ASSERT_EQUAL((unsigned int)3, req.getRedirectCount()); } void RequestTest::testRedirectUrl2() {