From 5ccd5b6953c95986407a735a41e8c36cf05944d1 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Wed, 4 Nov 2015 00:28:44 +0900
Subject: [PATCH] Remove content-length and content-range if transfer-encoding
 is given

See GH-473
---
 src/HttpHeader.cc               |  5 +++++
 src/HttpHeader.h                |  2 ++
 src/HttpHeaderProcessor.cc      | 19 ++++++++++++++++++-
 src/HttpResponseCommand.cc      |  3 ++-
 test/HttpHeaderProcessorTest.cc | 22 ++++++++++++++++++++++
 test/HttpHeaderTest.cc          | 15 +++++++++++++++
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 6b4739b5..a9fe23a6 100644
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -53,6 +53,11 @@ void HttpHeader::put(int hdKey, const std::string& value)
   table_.insert(vt);
 }
 
+void HttpHeader::remove(int hdKey)
+{
+  table_.erase(hdKey);
+}
+
 bool HttpHeader::defined(int hdKey) const
 {
   return table_.count(hdKey);
diff --git a/src/HttpHeader.h b/src/HttpHeader.h
index 897251b5..5697af9e 100644
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -106,6 +106,8 @@ public:
             std::multimap<int, std::string>::const_iterator>
   equalRange(int hdKey) const;
 
+  void remove(int hdKey);
+
   Range getRange() const;
 
   int getStatusCode() const;
diff --git a/src/HttpHeaderProcessor.cc b/src/HttpHeaderProcessor.cc
index 5a5fab35..52dfc758 100644
--- a/src/HttpHeaderProcessor.cc
+++ b/src/HttpHeaderProcessor.cc
@@ -446,7 +446,24 @@ fin:
 
   lastBytesProcessed_ = i;
   headers_.append(&data[0], &data[i]);
-  return state_ == HEADERS_COMPLETE;
+
+  if (state_ != HEADERS_COMPLETE) {
+    return false;
+  }
+
+  // If both transfer-encoding and (content-length or content-range)
+  // are present, delete content-length and content-range.  RFC 7230
+  // says that sender must not send both transfer-encoding and
+  // content-length.  If both present, transfer-encoding overrides
+  // content-length.  There is no text about transfer-encoding and
+  // content-range.  But there is no reason to send transfer-encoding
+  // when range is set.
+  if (result_->defined(HttpHeader::TRANSFER_ENCODING)) {
+    result_->remove(HttpHeader::CONTENT_LENGTH);
+    result_->remove(HttpHeader::CONTENT_RANGE);
+  }
+
+  return true;
 }
 
 bool HttpHeaderProcessor::parse(const std::string& data)
diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc
index b259cbeb..d8dfffe9 100644
--- a/src/HttpResponseCommand.cc
+++ b/src/HttpResponseCommand.cc
@@ -263,7 +263,8 @@ bool HttpResponseCommand::executeInternal()
     updateLastModifiedTime(httpResponse->getLastModifiedTime());
 
     // If both transfer-encoding and total length is specified, we
-    // assume we can do segmented downloading
+    // should have ignored total length.  In this case, we can not do
+    // segmented downloading
     if (totalLength == 0 || shouldInflateContentEncoding(httpResponse.get())) {
       // we ignore content-length when inflate is required
       fe->setLength(0);
diff --git a/test/HttpHeaderProcessorTest.cc b/test/HttpHeaderProcessorTest.cc
index d29aa16f..f0c61d72 100644
--- a/test/HttpHeaderProcessorTest.cc
+++ b/test/HttpHeaderProcessorTest.cc
@@ -22,6 +22,7 @@ class HttpHeaderProcessorTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testGetHttpResponseHeader_statusOnly);
   CPPUNIT_TEST(testGetHttpResponseHeader_insufficientStatusLength);
   CPPUNIT_TEST(testGetHttpResponseHeader_nameStartsWs);
+  CPPUNIT_TEST(testGetHttpResponseHeader_teAndCl);
   CPPUNIT_TEST(testBeyondLimit);
   CPPUNIT_TEST(testGetHeaderString);
   CPPUNIT_TEST(testGetHttpRequestHeader);
@@ -37,6 +38,7 @@ public:
   void testGetHttpResponseHeader_statusOnly();
   void testGetHttpResponseHeader_insufficientStatusLength();
   void testGetHttpResponseHeader_nameStartsWs();
+  void testGetHttpResponseHeader_teAndCl();
   void testBeyondLimit();
   void testGetHeaderString();
   void testGetHttpRequestHeader();
@@ -210,6 +212,26 @@ void HttpHeaderProcessorTest::testGetHttpResponseHeader_nameStartsWs()
   }
 }
 
+void HttpHeaderProcessorTest::testGetHttpResponseHeader_teAndCl()
+{
+  HttpHeaderProcessor proc(HttpHeaderProcessor::CLIENT_PARSER);
+
+  std::string hd =
+    "HTTP/1.1 200\r\n"
+    "Content-Length: 200\r\n"
+    "Transfer-Encoding: chunked\r\n"
+    "Content-Range: 1-200/300\r\n"
+    "\r\n";
+
+  CPPUNIT_ASSERT(proc.parse(hd));
+
+  auto httpHeader = proc.getResult();
+  CPPUNIT_ASSERT_EQUAL(std::string("chunked"),
+                       httpHeader->find(HttpHeader::TRANSFER_ENCODING));
+  CPPUNIT_ASSERT(!httpHeader->defined(HttpHeader::CONTENT_LENGTH));
+  CPPUNIT_ASSERT(!httpHeader->defined(HttpHeader::CONTENT_RANGE));
+}
+
 void HttpHeaderProcessorTest::testBeyondLimit()
 {
   HttpHeaderProcessor proc(HttpHeaderProcessor::CLIENT_PARSER);
diff --git a/test/HttpHeaderTest.cc b/test/HttpHeaderTest.cc
index 9573da23..80bb7aa6 100644
--- a/test/HttpHeaderTest.cc
+++ b/test/HttpHeaderTest.cc
@@ -14,6 +14,7 @@ class HttpHeaderTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testFindAll);
   CPPUNIT_TEST(testClearField);
   CPPUNIT_TEST(testFieldContains);
+  CPPUNIT_TEST(testRemove);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -21,6 +22,7 @@ public:
   void testFindAll();
   void testClearField();
   void testFieldContains();
+  void testRemove();
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION( HttpHeaderTest );
@@ -167,4 +169,17 @@ void HttpHeaderTest::testFieldContains()
   CPPUNIT_ASSERT(!h.fieldContains(HttpHeader::SEC_WEBSOCKET_VERSION, "6"));
 }
 
+void HttpHeaderTest::testRemove()
+{
+  HttpHeader h;
+  h.put(HttpHeader::CONNECTION, "close");
+  h.put(HttpHeader::TRANSFER_ENCODING, "chunked");
+  h.put(HttpHeader::TRANSFER_ENCODING, "gzip");
+
+  h.remove(HttpHeader::TRANSFER_ENCODING);
+
+  CPPUNIT_ASSERT(!h.defined(HttpHeader::TRANSFER_ENCODING));
+  CPPUNIT_ASSERT(h.defined(HttpHeader::CONNECTION));
+}
+
 } // namespace aria2