From ec4b7297047ec181860049d66e71c03317f1fdc3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 5 Feb 2014 00:32:08 +0900 Subject: [PATCH] Better handling of 30X HTTP status codes Reference: http://greenbytes.de/tech/tc/httpredirects/ --- src/HttpResponse.cc | 58 ++++++++++++++++++++----------------- src/HttpResponseCommand.cc | 9 +++--- test/HttpResponseTest.cc | 59 ++++++++++++++++++++++++++++---------- 3 files changed, 80 insertions(+), 46 deletions(-) diff --git a/src/HttpResponse.cc b/src/HttpResponse.cc index 7267e51a..27eb500f 100644 --- a/src/HttpResponse.cc +++ b/src/HttpResponse.cc @@ -75,27 +75,9 @@ HttpResponse::HttpResponse() void HttpResponse::validateResponse() const { int statusCode = getStatusCode(); - if (statusCode >= 400) { - return; - } - - if (statusCode == 304) { - if (!httpRequest_->conditionalRequest()) { - throw DL_ABORT_EX2("Got 304 without If-Modified-Since or If-None-Match", - error_code::HTTP_PROTOCOL_ERROR); - } - } - else if (statusCode == 301 || - statusCode == 302 || - statusCode == 303 || - statusCode == 307) { - if (!httpHeader_->defined(HttpHeader::LOCATION)) { - throw DL_ABORT_EX2(fmt(EX_LOCATION_HEADER_REQUIRED, statusCode), - error_code::HTTP_PROTOCOL_ERROR); - } - return; - } - else if (statusCode == 200 || statusCode == 206) { + switch(statusCode) { + case 200: // OK + case 206: // Partial Content if (!httpHeader_->defined(HttpHeader::TRANSFER_ENCODING)) { // compare the received range against the requested range auto responseRange = httpHeader_->getRange(); @@ -110,11 +92,26 @@ void HttpResponse::validateResponse() const error_code::CANNOT_RESUME); } } + return; + case 304: // Not Modified + if (!httpRequest_->conditionalRequest()) { + throw DL_ABORT_EX2("Got 304 without If-Modified-Since or If-None-Match", + error_code::HTTP_PROTOCOL_ERROR); + } + return; + case 300: // Multiple Choices + case 301: // Moved Permanently + case 302: // Found + case 303: // See Other + case 307: // Temporary Redirect + case 308: // Permanent Redirect + return; } - else { - throw DL_ABORT_EX2(fmt("Unexpected status %d", statusCode), - error_code::HTTP_PROTOCOL_ERROR); + if (statusCode >= 400) { + return; } + throw DL_ABORT_EX2(fmt("Unexpected status %d", statusCode), + error_code::HTTP_PROTOCOL_ERROR); } std::string HttpResponse::determineFilename() const @@ -151,9 +148,16 @@ void HttpResponse::retrieveCookie() bool HttpResponse::isRedirect() const { - auto code = getStatusCode(); - return (301 == code || 302 == code || 303 == code || 307 == code) && - httpHeader_->defined(HttpHeader::LOCATION); + switch(getStatusCode()) { + case 300: // Multiple Choices + case 301: // Moved Permanently + case 302: // Found + case 303: // See Other + case 307: // Temporary Redirect + case 308: // Permanent Redirect + return httpHeader_->defined(HttpHeader::LOCATION); + } + return false; } void HttpResponse::processRedirect() diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc index 4c047a27..622736d3 100644 --- a/src/HttpResponseCommand.cc +++ b/src/HttpResponseCommand.cc @@ -231,10 +231,11 @@ bool HttpResponseCommand::executeInternal() #endif // ENABLE_MESSAGE_DIGEST } - if (statusCode >= 300) { - if (statusCode == 404) { - grp->increaseAndValidateFileNotFoundCount(); - } + if (statusCode == 404) { + grp->increaseAndValidateFileNotFoundCount(); + return skipResponseBody(std::move(httpResponse)); + } + if (statusCode >= 400 || statusCode == 304 || httpResponse->isRedirect()) { return skipResponseBody(std::move(httpResponse)); } diff --git a/test/HttpResponseTest.cc b/test/HttpResponseTest.cc index 3e5ec6ef..eca1df08 100644 --- a/test/HttpResponseTest.cc +++ b/test/HttpResponseTest.cc @@ -220,16 +220,49 @@ void HttpResponseTest::testGetRedirectURI_with_Location() void HttpResponseTest::testIsRedirect() { HttpResponse httpResponse; - auto httpHeader = make_unique(); - httpHeader->setStatusCode(200); - httpHeader->put(HttpHeader::LOCATION, - "http://localhost/download/aria2-1.0.0.tar.bz2"); + httpResponse.setHttpHeader(make_unique()); - httpResponse.setHttpHeader(std::move(httpHeader)); + httpResponse.getHttpHeader()->setStatusCode(301); CPPUNIT_ASSERT(!httpResponse.isRedirect()); + httpResponse.getHttpHeader()->setStatusCode(200); + CPPUNIT_ASSERT(!httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->put + (HttpHeader::LOCATION, + "http://localhost/download/aria2-1.0.0.tar.bz2"); + + CPPUNIT_ASSERT(!httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(300); + CPPUNIT_ASSERT(httpResponse.isRedirect()); + httpResponse.getHttpHeader()->setStatusCode(301); CPPUNIT_ASSERT(httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(302); + CPPUNIT_ASSERT(httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(303); + CPPUNIT_ASSERT(httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(304); + CPPUNIT_ASSERT(!httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(305); + CPPUNIT_ASSERT(!httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(306); + CPPUNIT_ASSERT(!httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(307); + CPPUNIT_ASSERT(httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(308); + CPPUNIT_ASSERT(httpResponse.isRedirect()); + + httpResponse.getHttpHeader()->setStatusCode(309); + CPPUNIT_ASSERT(!httpResponse.isRedirect()); } void HttpResponseTest::testIsTransferEncodingSpecified() @@ -329,19 +362,15 @@ void HttpResponseTest::testValidateResponse() httpResponse.setHttpHeader(make_unique()); httpResponse.getHttpHeader()->setStatusCode(301); - try { + // It is fine without Location header + httpResponse.validateResponse(); + + httpResponse.getHttpHeader()->setStatusCode(201); + try{ httpResponse.validateResponse(); CPPUNIT_FAIL("exception must be thrown."); } catch(Exception& e) { - } - - httpResponse.getHttpHeader()->put - (HttpHeader::LOCATION, - "http://localhost/archives/aria2-1.0.0.tar.bz2"); - try { - httpResponse.validateResponse(); - } catch(Exception& e) { - CPPUNIT_FAIL("exception must not be thrown."); + // success } }