From 1a99bacfe3e19df6e4346bf46d7336852419e99d Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 22 May 2020 16:30:43 +0400 Subject: [PATCH 01/33] Byte ranges are inclusive The second component of a byte range, if present, designates the index of the last byte to be included in the partial response. --- src/server/response.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index cdd723a26..ab7923d1a 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -43,7 +43,7 @@ int get_range_len(const kiwix::Entry& entry, RequestContext::ByteRange range) { return range.second == -1 ? entry.getSize() - range.first - : range.second - range.first; + : range.second - range.first + 1; } } // unnamed namespace From 0a30a77c080616b20383423f4211f41acfd5c231 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 22 May 2020 16:46:38 +0400 Subject: [PATCH 02/33] Handling of out of bound byte ranges --- src/server/response.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index ab7923d1a..0b4cb522d 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -41,9 +41,10 @@ bool is_compressible_mime_type(const std::string& mimeType) int get_range_len(const kiwix::Entry& entry, RequestContext::ByteRange range) { + const int entrySize = entry.getSize(); return range.second == -1 - ? entry.getSize() - range.first - : range.second - range.first + 1; + ? entrySize - range.first + : std::min(range.second + 1, entrySize) - range.first; } } // unnamed namespace From 2a35a86de6bdbfb5660fee8616ea2b6bdcfcb0b3 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 22 May 2020 16:49:35 +0400 Subject: [PATCH 03/33] Fixed the size value used creating a response In case of a partial response the size of the response is different from the served entry size. --- src/server/response.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 0b4cb522d..410e9ebb1 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -241,7 +241,7 @@ Response::create_redirection_mhd_response() const MHD_Response* Response::create_entry_mhd_response() const { - MHD_Response* response = MHD_create_response_from_callback(m_entry.getSize(), + MHD_Response* response = MHD_create_response_from_callback(m_lenRange, 16384, callback_reader_from_entry, new RunningResponse(m_entry, m_startRange), From de37489c5357da79ae2b70a432b031e2e5d163bf Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 22 May 2020 17:12:54 +0400 Subject: [PATCH 04/33] Range header starts with a unit spec After this commit valid ranges of the form "bytes=firstbyte-lastbyte" should be handled correctly. --- src/server/request_context.cpp | 35 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 37d490bf9..1ea184ebf 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -20,6 +20,7 @@ #include "request_context.h" +#include "tools/stringTools.h" #include #include #include @@ -87,22 +88,26 @@ RequestContext::RequestContext(struct MHD_Connection* connection, /*Check if range is requested. */ try { - auto range = get_header(MHD_HTTP_HEADER_RANGE); - int start = 0; - int end = -1; - std::istringstream iss(range); - char c; + std::string range = get_header(MHD_HTTP_HEADER_RANGE); + const std::string byteUnitSpec("bytes="); + if ( kiwix::startsWith(range, byteUnitSpec) ) { + range.erase(0, byteUnitSpec.size()); + int start = 0; + int end = -1; + std::istringstream iss(range); + char c; - iss >> start >> c; - if (iss.good() && c=='-') { - iss >> end; - if (iss.fail()) { - // Something went wrong will extracting. - end = -1; - } - if (iss.eof()) { - accept_range = true; - range_pair = std::pair(start, end); + iss >> start >> c; + if (iss.good() && c=='-') { + iss >> end; + if (iss.fail()) { + // Something went wrong will extracting. + end = -1; + } + if (iss.eof()) { + accept_range = true; + range_pair = std::pair(start, end); + } } } } catch (const std::out_of_range&) {} From bd2d0bc489a39c5e5083f86dfc7c77597f89217f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 22 May 2020 17:39:00 +0400 Subject: [PATCH 05/33] Unit-test for valid single range requests --- test/server.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/server.cpp b/test/server.cpp index 6ed18e409..d232f4542 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -408,3 +408,32 @@ TEST_F(ServerTest, IfNoneMatchRequestsWithMismatchingETagResultIn200Responses) EXPECT_EQ(200, g2->status); } } + +TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) +{ + const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + const auto full = zfs1_->GET(url); + + { + const auto p = zfs1_->GET(url, { {"Range", "bytes=0-100000"} } ); + EXPECT_EQ(206, p->status); + EXPECT_EQ(full->body, p->body); + EXPECT_EQ("bytes 0-20076/20077", p->get_header_value("Content-Range")); + } + + { + const auto p = zfs1_->GET(url, { {"Range", "bytes=0-10"} } ); + EXPECT_EQ(206, p->status); + EXPECT_EQ("bytes 0-10/20077", p->get_header_value("Content-Range")); + EXPECT_EQ(11, p->body.size()); + EXPECT_EQ(full->body.substr(0, 11), p->body); + } + + { + const auto p = zfs1_->GET(url, { {"Range", "bytes=123-456"} } ); + EXPECT_EQ(206, p->status); + EXPECT_EQ("bytes 123-456/20077", p->get_header_value("Content-Range")); + EXPECT_EQ(334, p->body.size()); + EXPECT_EQ(full->body.substr(123, 334), p->body); + } +} From c39fce88396049589592327295a53538df127fbb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 17:09:51 +0400 Subject: [PATCH 06/33] RequestContext::parse_byte_range() --- src/server/request_context.cpp | 8 ++++++-- src/server/request_context.h | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 1ea184ebf..a242b742f 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -86,9 +86,14 @@ RequestContext::RequestContext(struct MHD_Connection* connection, (get_header(MHD_HTTP_HEADER_ACCEPT_ENCODING).find("deflate") != std::string::npos); } catch (const std::out_of_range&) {} - /*Check if range is requested. */ try { std::string range = get_header(MHD_HTTP_HEADER_RANGE); + parse_byte_range(range); + } catch (const std::out_of_range&) {} +} + +void RequestContext::parse_byte_range(std::string range) +{ const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(range, byteUnitSpec) ) { range.erase(0, byteUnitSpec.size()); @@ -110,7 +115,6 @@ RequestContext::RequestContext(struct MHD_Connection* connection, } } } - } catch (const std::out_of_range&) {} } RequestContext::~RequestContext() diff --git a/src/server/request_context.h b/src/server/request_context.h index d58040f07..1a4163919 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -86,6 +86,9 @@ class RequestContext { bool can_compress() const { return acceptEncodingDeflate; } + private: // functions + void parse_byte_range(std::string range); + private: // data std::string full_url; std::string url; From a0f7f32570a67a3618c9e940c604a8b2db6e14aa Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 17:11:26 +0400 Subject: [PATCH 07/33] Re-ordered function definitions --- src/server/request_context.cpp | 50 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index a242b742f..996a7cc9c 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -92,34 +92,34 @@ RequestContext::RequestContext(struct MHD_Connection* connection, } catch (const std::out_of_range&) {} } -void RequestContext::parse_byte_range(std::string range) -{ - const std::string byteUnitSpec("bytes="); - if ( kiwix::startsWith(range, byteUnitSpec) ) { - range.erase(0, byteUnitSpec.size()); - int start = 0; - int end = -1; - std::istringstream iss(range); - char c; - - iss >> start >> c; - if (iss.good() && c=='-') { - iss >> end; - if (iss.fail()) { - // Something went wrong will extracting. - end = -1; - } - if (iss.eof()) { - accept_range = true; - range_pair = std::pair(start, end); - } - } - } -} - RequestContext::~RequestContext() {} +void RequestContext::parse_byte_range(std::string range) +{ + const std::string byteUnitSpec("bytes="); + if ( kiwix::startsWith(range, byteUnitSpec) ) { + range.erase(0, byteUnitSpec.size()); + int start = 0; + int end = -1; + std::istringstream iss(range); + char c; + + iss >> start >> c; + if (iss.good() && c=='-') { + iss >> end; + if (iss.fail()) { + // Something went wrong while extracting + end = -1; + } + if (iss.eof()) { + accept_range = true; + range_pair = std::pair(start, end); + } + } + } +} + int RequestContext::fill_header(void *__this, enum MHD_ValueKind kind, const char *key, const char *value) From e6a86c02ae2f2a7c8e3d511c731c992b770e4165 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 17:15:42 +0400 Subject: [PATCH 08/33] Got rid of RequestContext::accept_range --- src/server/request_context.cpp | 6 ++---- src/server/request_context.h | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 996a7cc9c..573a36aaa 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -75,7 +75,6 @@ RequestContext::RequestContext(struct MHD_Connection* connection, version(version), requestIndex(s_requestIndex++), acceptEncodingDeflate(false), - accept_range(false), range_pair(0, -1) { MHD_get_connection_values(connection, MHD_HEADER_KIND, &RequestContext::fill_header, this); @@ -113,7 +112,6 @@ void RequestContext::parse_byte_range(std::string range) end = -1; } if (iss.eof()) { - accept_range = true; range_pair = std::pair(start, end); } } @@ -155,7 +153,7 @@ void RequestContext::print_debug_info() const { printf("full_url: %s\n", full_url.c_str()); printf("url : %s\n", url.c_str()); printf("acceptEncodingDeflate : %d\n", acceptEncodingDeflate); - printf("has_range : %d\n", accept_range); + printf("has_range : %d\n", has_range()); printf("is_valid_url : %d\n", is_valid_url()); printf(".............\n"); } @@ -198,7 +196,7 @@ bool RequestContext::is_valid_url() const { } bool RequestContext::has_range() const { - return accept_range; + return range_pair.first <= range_pair.second; } std::pair RequestContext::get_range() const { diff --git a/src/server/request_context.h b/src/server/request_context.h index 1a4163919..ecd38d2b9 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -98,7 +98,6 @@ class RequestContext { bool acceptEncodingDeflate; - bool accept_range; ByteRange range_pair; std::map headers; std::map arguments; From 81c38d6b2bae982981c6cefbd72fa21ef4e56d40 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 18:53:16 +0400 Subject: [PATCH 09/33] parse_byte_range() without side-effects --- src/server/request_context.cpp | 11 ++++++----- src/server/request_context.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 573a36aaa..0e73345a3 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -86,16 +86,16 @@ RequestContext::RequestContext(struct MHD_Connection* connection, } catch (const std::out_of_range&) {} try { - std::string range = get_header(MHD_HTTP_HEADER_RANGE); - parse_byte_range(range); + range_pair = parse_byte_range(get_header(MHD_HTTP_HEADER_RANGE)); } catch (const std::out_of_range&) {} } RequestContext::~RequestContext() {} -void RequestContext::parse_byte_range(std::string range) +RequestContext::ByteRange RequestContext::parse_byte_range(std::string range) { + ByteRange byteRange{0, -1}; const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(range, byteUnitSpec) ) { range.erase(0, byteUnitSpec.size()); @@ -112,10 +112,11 @@ void RequestContext::parse_byte_range(std::string range) end = -1; } if (iss.eof()) { - range_pair = std::pair(start, end); + byteRange = ByteRange(start, end); } } } + return byteRange; } @@ -199,7 +200,7 @@ bool RequestContext::has_range() const { return range_pair.first <= range_pair.second; } -std::pair RequestContext::get_range() const { +RequestContext::ByteRange RequestContext::get_range() const { return range_pair; } diff --git a/src/server/request_context.h b/src/server/request_context.h index ecd38d2b9..ea30d1bbe 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -87,7 +87,7 @@ class RequestContext { bool can_compress() const { return acceptEncodingDeflate; } private: // functions - void parse_byte_range(std::string range); + static ByteRange parse_byte_range(std::string range); private: // data std::string full_url; From 54db6049b774c18500ebf702a42e090e8ab98be0 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 18:58:19 +0400 Subject: [PATCH 10/33] Byte-range parsing not exposed in the header file --- src/server/request_context.cpp | 56 ++++++++++++++++++---------------- src/server/request_context.h | 3 -- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 0e73345a3..1d467669e 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -62,6 +62,35 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) } } +using ByteRange = RequestContext::ByteRange; + +ByteRange parse_byte_range(std::string range) +{ + ByteRange byteRange{0, -1}; + const std::string byteUnitSpec("bytes="); + if ( kiwix::startsWith(range, byteUnitSpec) ) { + range.erase(0, byteUnitSpec.size()); + int start = 0; + int end = -1; + std::istringstream iss(range); + char c; + + iss >> start >> c; + if (iss.good() && c=='-') { + iss >> end; + if (iss.fail()) { + // Something went wrong while extracting + end = -1; + } + if (iss.eof()) { + byteRange = ByteRange(start, end); + } + } + } + return byteRange; +} + + } // unnamed namespace RequestContext::RequestContext(struct MHD_Connection* connection, @@ -93,33 +122,6 @@ RequestContext::RequestContext(struct MHD_Connection* connection, RequestContext::~RequestContext() {} -RequestContext::ByteRange RequestContext::parse_byte_range(std::string range) -{ - ByteRange byteRange{0, -1}; - const std::string byteUnitSpec("bytes="); - if ( kiwix::startsWith(range, byteUnitSpec) ) { - range.erase(0, byteUnitSpec.size()); - int start = 0; - int end = -1; - std::istringstream iss(range); - char c; - - iss >> start >> c; - if (iss.good() && c=='-') { - iss >> end; - if (iss.fail()) { - // Something went wrong while extracting - end = -1; - } - if (iss.eof()) { - byteRange = ByteRange(start, end); - } - } - } - return byteRange; -} - - int RequestContext::fill_header(void *__this, enum MHD_ValueKind kind, const char *key, const char *value) { diff --git a/src/server/request_context.h b/src/server/request_context.h index ea30d1bbe..13744b7fb 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -86,9 +86,6 @@ class RequestContext { bool can_compress() const { return acceptEncodingDeflate; } - private: // functions - static ByteRange parse_byte_range(std::string range); - private: // data std::string full_url; std::string url; From 3fba8c20a015043702c1afd0d9af4865a562a446 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 19:59:47 +0400 Subject: [PATCH 11/33] Converted RequestContext::ByteRange to a class Also renamed the `range_pair` data member of `RequestContext` to `byteRange_` --- src/server/request_context.cpp | 8 ++++---- src/server/request_context.h | 15 +++++++++++++-- src/server/response.cpp | 10 +++++----- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 1d467669e..fb54ca9ef 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -104,7 +104,7 @@ RequestContext::RequestContext(struct MHD_Connection* connection, version(version), requestIndex(s_requestIndex++), acceptEncodingDeflate(false), - range_pair(0, -1) + byteRange_() { MHD_get_connection_values(connection, MHD_HEADER_KIND, &RequestContext::fill_header, this); MHD_get_connection_values(connection, MHD_GET_ARGUMENT_KIND, &RequestContext::fill_argument, this); @@ -115,7 +115,7 @@ RequestContext::RequestContext(struct MHD_Connection* connection, } catch (const std::out_of_range&) {} try { - range_pair = parse_byte_range(get_header(MHD_HTTP_HEADER_RANGE)); + byteRange_ = parse_byte_range(get_header(MHD_HTTP_HEADER_RANGE)); } catch (const std::out_of_range&) {} } @@ -199,11 +199,11 @@ bool RequestContext::is_valid_url() const { } bool RequestContext::has_range() const { - return range_pair.first <= range_pair.second; + return byteRange_.first() <= byteRange_.last(); } RequestContext::ByteRange RequestContext::get_range() const { - return range_pair; + return byteRange_; } template<> diff --git a/src/server/request_context.h b/src/server/request_context.h index 13744b7fb..100e2fa35 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -52,7 +52,18 @@ class IndexError: public std::runtime_error {}; class RequestContext { public: // types - typedef std::pair ByteRange; + class ByteRange { + public: // functions + ByteRange() : ByteRange(0, -1) {} + ByteRange(int64_t first, int64_t last) : first_(first), last_(last) {} + + int64_t first() const { return first_; } + int64_t last() const { return last_; } + + private: // data + int64_t first_; + int64_t last_; + }; public: // functions RequestContext(struct MHD_Connection* connection, @@ -95,7 +106,7 @@ class RequestContext { bool acceptEncodingDeflate; - ByteRange range_pair; + ByteRange byteRange_; std::map headers; std::map arguments; diff --git a/src/server/response.cpp b/src/server/response.cpp index 410e9ebb1..448753dc9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -41,10 +41,10 @@ bool is_compressible_mime_type(const std::string& mimeType) int get_range_len(const kiwix::Entry& entry, RequestContext::ByteRange range) { - const int entrySize = entry.getSize(); - return range.second == -1 - ? entrySize - range.first - : std::min(range.second + 1, entrySize) - range.first; + const int64_t entrySize = entry.getSize(); + return range.last() == -1 + ? entrySize - range.first() + : std::min(range.last() + 1, entrySize) - range.first(); } } // unnamed namespace @@ -330,7 +330,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_compress(true); } else { const int range_len = get_range_len(entry, request.get_range()); - set_range_first(request.get_range().first); + set_range_first(request.get_range().first()); set_range_len(range_len); } } From 0c5bb3fcfe3863a9838b9bbb926e8a72bace10be Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 20:08:53 +0400 Subject: [PATCH 12/33] Moved ByteRange to a header file of its own --- src/server/byte_range.h | 42 ++++++++++++++++++++++++++++++++++ src/server/request_context.cpp | 4 +--- src/server/request_context.h | 16 ++----------- src/server/response.cpp | 2 +- 4 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 src/server/byte_range.h diff --git a/src/server/byte_range.h b/src/server/byte_range.h new file mode 100644 index 000000000..4579133d3 --- /dev/null +++ b/src/server/byte_range.h @@ -0,0 +1,42 @@ +/* + * Copyright 2020 Veloman Yunkan + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + + +#ifndef KIWIXLIB_SERVER_BYTE_RANGE_H +#define KIWIXLIB_SERVER_BYTE_RANGE_H + +namespace kiwix { + +class ByteRange +{ + public: // functions + ByteRange() : ByteRange(0, -1) {} + ByteRange(int64_t first, int64_t last) : first_(first), last_(last) {} + + int64_t first() const { return first_; } + int64_t last() const { return last_; } + + private: // data + int64_t first_; + int64_t last_; +}; + +} // namespace kiwix + +#endif //KIWIXLIB_SERVER_BYTE_RANGE_H diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index fb54ca9ef..963f75c34 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -62,8 +62,6 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) } } -using ByteRange = RequestContext::ByteRange; - ByteRange parse_byte_range(std::string range) { ByteRange byteRange{0, -1}; @@ -202,7 +200,7 @@ bool RequestContext::has_range() const { return byteRange_.first() <= byteRange_.last(); } -RequestContext::ByteRange RequestContext::get_range() const { +ByteRange RequestContext::get_range() const { return byteRange_; } diff --git a/src/server/request_context.h b/src/server/request_context.h index 100e2fa35..3256e30e5 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -27,6 +27,8 @@ #include #include +#include "byte_range.h" + extern "C" { #include } @@ -51,20 +53,6 @@ class IndexError: public std::runtime_error {}; class RequestContext { - public: // types - class ByteRange { - public: // functions - ByteRange() : ByteRange(0, -1) {} - ByteRange(int64_t first, int64_t last) : first_(first), last_(last) {} - - int64_t first() const { return first_; } - int64_t last() const { return last_; } - - private: // data - int64_t first_; - int64_t last_; - }; - public: // functions RequestContext(struct MHD_Connection* connection, std::string rootLocation, diff --git a/src/server/response.cpp b/src/server/response.cpp index 448753dc9..23da5c12c 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -39,7 +39,7 @@ bool is_compressible_mime_type(const std::string& mimeType) || mimeType.find("application/json") != string::npos; } -int get_range_len(const kiwix::Entry& entry, RequestContext::ByteRange range) +int get_range_len(const kiwix::Entry& entry, ByteRange range) { const int64_t entrySize = entry.getSize(); return range.last() == -1 From d111a40ce866dce91bedfb9d8c5d6602b79df9eb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 23 May 2020 20:35:22 +0400 Subject: [PATCH 13/33] Response::m_byteRange --- src/server/byte_range.h | 1 + src/server/response.cpp | 26 ++++++++++++-------------- src/server/response.h | 6 ++---- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/server/byte_range.h b/src/server/byte_range.h index 4579133d3..eb61dde45 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -31,6 +31,7 @@ class ByteRange int64_t first() const { return first_; } int64_t last() const { return last_; } + int64_t length() const { return last_ + 1 - first_; } private: // data int64_t first_; diff --git a/src/server/response.cpp b/src/server/response.cpp index 23da5c12c..011e8a902 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -39,12 +39,13 @@ bool is_compressible_mime_type(const std::string& mimeType) || mimeType.find("application/json") != string::npos; } -int get_range_len(const kiwix::Entry& entry, ByteRange range) +ByteRange resolve_byte_range(const kiwix::Entry& entry, ByteRange range) { const int64_t entrySize = entry.getSize(); - return range.last() == -1 - ? entrySize - range.first() - : std::min(range.last() + 1, entrySize) - range.first(); + const int64_t resolved_last = range.last() < 0 + ? entrySize - 1 + : std::min(entrySize-1, range.last()); + return ByteRange(range.first(), resolved_last); } } // unnamed namespace @@ -59,9 +60,7 @@ Response::Response(const std::string& root, bool verbose, bool withTaskbar, bool m_withLibraryButton(withLibraryButton), m_blockExternalLinks(blockExternalLinks), m_addTaskbar(false), - m_bookName(""), - m_startRange(0), - m_lenRange(0) + m_bookName("") { } @@ -241,23 +240,24 @@ Response::create_redirection_mhd_response() const MHD_Response* Response::create_entry_mhd_response() const { - MHD_Response* response = MHD_create_response_from_callback(m_lenRange, + const auto content_length = m_byteRange.length(); + MHD_Response* response = MHD_create_response_from_callback(content_length, 16384, callback_reader_from_entry, - new RunningResponse(m_entry, m_startRange), + new RunningResponse(m_entry, m_byteRange.first()), callback_free_response); MHD_add_response_header(response, MHD_HTTP_HEADER_CONTENT_TYPE, m_mimeType.c_str()); MHD_add_response_header(response, MHD_HTTP_HEADER_ACCEPT_RANGES, "bytes"); std::ostringstream oss; - oss << "bytes " << m_startRange << "-" << m_startRange + m_lenRange - 1 + oss << "bytes " << m_byteRange.first() << "-" << m_byteRange.last() << "/" << m_entry.getSize(); MHD_add_response_header(response, MHD_HTTP_HEADER_CONTENT_RANGE, oss.str().c_str()); MHD_add_response_header(response, - MHD_HTTP_HEADER_CONTENT_LENGTH, kiwix::to_string(m_lenRange).c_str()); + MHD_HTTP_HEADER_CONTENT_LENGTH, kiwix::to_string(content_length).c_str()); return response; } @@ -329,9 +329,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_content(content); set_compress(true); } else { - const int range_len = get_range_len(entry, request.get_range()); - set_range_first(request.get_range().first()); - set_range_len(range_len); + m_byteRange = resolve_byte_range(entry, request.get_range()); } } diff --git a/src/server/response.h b/src/server/response.h index bd15f9543..9fba5a191 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -24,6 +24,7 @@ #include #include +#include "byte_range.h" #include "entry.h" #include "etag.h" @@ -61,8 +62,6 @@ class Response { void set_etag(const ETag& etag) { m_etag = etag; } void set_compress(bool compress) { m_compress = compress; } void set_taskbar(const std::string& bookName, const std::string& bookTitle); - void set_range_first(uint64_t start) { m_startRange = start; } - void set_range_len(uint64_t len) { m_lenRange = len; } int getReturnCode() const { return m_returnCode; } std::string get_mimeType() const { return m_mimeType; } @@ -93,8 +92,7 @@ class Response { bool m_addTaskbar; std::string m_bookName; std::string m_bookTitle; - uint64_t m_startRange; - uint64_t m_lenRange; + ByteRange m_byteRange; ETag m_etag; }; From 67294217a887a17472e5d753014d3d3b7a3bba08 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 16:23:44 +0400 Subject: [PATCH 14/33] ByteRange::Kind --- src/server/byte_range.h | 28 +++++++++++++++++++++++++--- src/server/request_context.cpp | 6 +++--- src/server/response.cpp | 10 ++++++++-- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/server/byte_range.h b/src/server/byte_range.h index eb61dde45..b9719d8b9 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -25,15 +25,37 @@ namespace kiwix { class ByteRange { - public: // functions - ByteRange() : ByteRange(0, -1) {} - ByteRange(int64_t first, int64_t last) : first_(first), last_(last) {} + public: // types + enum Kind { + // No byte-range was present in the request + NONE, + // This byte-range has been parsed from request + PARSED, + + // This is a response to a regular request + RESOLVED_FULL_CONTENT, + + // This is a response to a range request + RESOLVED_PARTIAL_CONTENT + }; + + public: // functions + ByteRange() : kind_(NONE), first_(0), last_(-1) {} + + ByteRange(Kind kind, int64_t first, int64_t last) + : kind_(kind) + , first_(first) + , last_(last) + {} + + Kind kind() const { return kind_; } int64_t first() const { return first_; } int64_t last() const { return last_; } int64_t length() const { return last_ + 1 - first_; } private: // data + Kind kind_; int64_t first_; int64_t last_; }; diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 963f75c34..caa985b5b 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -64,7 +64,7 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) ByteRange parse_byte_range(std::string range) { - ByteRange byteRange{0, -1}; + ByteRange byteRange; const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(range, byteUnitSpec) ) { range.erase(0, byteUnitSpec.size()); @@ -81,7 +81,7 @@ ByteRange parse_byte_range(std::string range) end = -1; } if (iss.eof()) { - byteRange = ByteRange(start, end); + byteRange = ByteRange(ByteRange::PARSED, start, end); } } } @@ -197,7 +197,7 @@ bool RequestContext::is_valid_url() const { } bool RequestContext::has_range() const { - return byteRange_.first() <= byteRange_.last(); + return byteRange_.kind() == ByteRange::PARSED; } ByteRange RequestContext::get_range() const { diff --git a/src/server/response.cpp b/src/server/response.cpp index 011e8a902..637559bb2 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -42,10 +42,14 @@ bool is_compressible_mime_type(const std::string& mimeType) ByteRange resolve_byte_range(const kiwix::Entry& entry, ByteRange range) { const int64_t entrySize = entry.getSize(); + + if ( range.kind() == ByteRange::NONE ) + return ByteRange(ByteRange::RESOLVED_FULL_CONTENT, 0, entrySize-1); + const int64_t resolved_last = range.last() < 0 ? entrySize - 1 : std::min(entrySize-1, range.last()); - return ByteRange(range.first(), resolved_last); + return ByteRange(ByteRange::RESOLVED_PARTIAL_CONTENT, range.first(), resolved_last); } } // unnamed namespace @@ -288,7 +292,7 @@ int Response::send(const RequestContext& request, MHD_Connection* connection) if ( ! etag.empty() ) MHD_add_response_header(response, MHD_HTTP_HEADER_ETAG, etag.c_str()); - if (m_returnCode == MHD_HTTP_OK && request.has_range()) + if (m_returnCode == MHD_HTTP_OK && m_byteRange.kind() == ByteRange::RESOLVED_PARTIAL_CONTENT) m_returnCode = MHD_HTTP_PARTIAL_CONTENT; if (m_verbose) @@ -322,6 +326,8 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_mimeType(mimeType); set_cacheable(); + // XXX: Which of the Accept-Encoding and Range headers has precedence if both + // XXX: are present? Can partial content be compressed? if ( is_compressible_mime_type(mimeType) ) { zim::Blob raw_content = entry.getBlob(); const std::string content = string(raw_content.data(), raw_content.size()); From 52f207eaa6542c43394938b177ae89edfc829453 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 16:37:01 +0400 Subject: [PATCH 15/33] Support for single-ended byte ranges --- src/server/request_context.cpp | 27 ++++++++++++++++++--------- src/server/response.cpp | 7 ++++++- test/server.cpp | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index caa985b5b..43259d8dc 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -26,6 +26,7 @@ #include #include #include +#include namespace kiwix { @@ -64,6 +65,8 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) ByteRange parse_byte_range(std::string range) { + const int64_t int64_max = std::numeric_limits::max(); + ByteRange byteRange; const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(range, byteUnitSpec) ) { @@ -73,15 +76,21 @@ ByteRange parse_byte_range(std::string range) std::istringstream iss(range); char c; - iss >> start >> c; - if (iss.good() && c=='-') { - iss >> end; - if (iss.fail()) { - // Something went wrong while extracting - end = -1; - } - if (iss.eof()) { - byteRange = ByteRange(ByteRange::PARSED, start, end); + iss >> start; + if ( start < 0 ) { + if ( iss.eof() ) + byteRange = ByteRange(ByteRange::PARSED, start, int64_max); + } else { + iss >> c; + if (iss.good() && c=='-') { + iss >> end; + if (iss.fail()) { + // Something went wrong while extracting + end = -1; + } + if (iss.eof()) { + byteRange = ByteRange(ByteRange::PARSED, start, end); + } } } } diff --git a/src/server/response.cpp b/src/server/response.cpp index 637559bb2..77d545a53 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -46,10 +46,15 @@ ByteRange resolve_byte_range(const kiwix::Entry& entry, ByteRange range) if ( range.kind() == ByteRange::NONE ) return ByteRange(ByteRange::RESOLVED_FULL_CONTENT, 0, entrySize-1); + const int64_t resolved_first = range.first() < 0 + ? std::max(int64_t(0), entrySize + range.first()) + : range.first(); + const int64_t resolved_last = range.last() < 0 ? entrySize - 1 : std::min(entrySize-1, range.last()); - return ByteRange(ByteRange::RESOLVED_PARTIAL_CONTENT, range.first(), resolved_last); + + return ByteRange(ByteRange::RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); } } // unnamed namespace diff --git a/test/server.cpp b/test/server.cpp index d232f4542..6031b0e65 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -436,4 +436,18 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ(334, p->body.size()); EXPECT_EQ(full->body.substr(123, 334), p->body); } + + { + const auto p = zfs1_->GET(url, { {"Range", "bytes=20000-"} } ); + EXPECT_EQ(206, p->status); + EXPECT_EQ(full->body.substr(20000), p->body); + EXPECT_EQ("bytes 20000-20076/20077", p->get_header_value("Content-Range")); + } + + { + const auto p = zfs1_->GET(url, { {"Range", "bytes=-100"} } ); + EXPECT_EQ(206, p->status); + EXPECT_EQ(full->body.substr(19977), p->body); + EXPECT_EQ("bytes 19977-20076/20077", p->get_header_value("Content-Range")); + } } From f3e79c6b4cca34d2dac50fbd1376d3348a4c8691 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 16:43:44 +0400 Subject: [PATCH 16/33] Introduced src/server/byte_range.cpp --- src/meson.build | 1 + src/server/byte_range.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/server/byte_range.h | 11 ++++------- 3 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 src/server/byte_range.cpp diff --git a/src/meson.build b/src/meson.build index 411f14f28..ddb188119 100644 --- a/src/meson.build +++ b/src/meson.build @@ -21,6 +21,7 @@ kiwix_sources = [ 'tools/otherTools.cpp', 'kiwixserve.cpp', 'name_mapper.cpp', + 'server/byte_range.cpp', 'server/etag.cpp', 'server/request_context.cpp', 'server/response.cpp' diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp new file mode 100644 index 000000000..5b3c98b2e --- /dev/null +++ b/src/server/byte_range.cpp @@ -0,0 +1,37 @@ +/* + * Copyright 2020 Veloman Yunkan + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + + +#include "byte_range.h" + +namespace kiwix { + +ByteRange::ByteRange() + : kind_(NONE) + , first_(0) + , last_(-1) +{} + +ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) + : kind_(kind) + , first_(first) + , last_(last) +{} + +} // namespace kiwix diff --git a/src/server/byte_range.h b/src/server/byte_range.h index b9719d8b9..4df6bd93c 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -21,6 +21,8 @@ #ifndef KIWIXLIB_SERVER_BYTE_RANGE_H #define KIWIXLIB_SERVER_BYTE_RANGE_H +#include + namespace kiwix { class ByteRange @@ -41,13 +43,8 @@ class ByteRange }; public: // functions - ByteRange() : kind_(NONE), first_(0), last_(-1) {} - - ByteRange(Kind kind, int64_t first, int64_t last) - : kind_(kind) - , first_(first) - , last_(last) - {} + ByteRange(); + ByteRange(Kind kind, int64_t first, int64_t last); Kind kind() const { return kind_; } int64_t first() const { return first_; } From 693905eb682fa244be4d9b1853e65108c6ddcac6 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 17:17:56 +0400 Subject: [PATCH 17/33] Default constructed ByteRange is a full range --- src/server/byte_range.cpp | 2 +- src/server/request_context.cpp | 28 ++++++++++------------------ src/server/response.cpp | 4 +--- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index 5b3c98b2e..5482914ab 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -25,7 +25,7 @@ namespace kiwix { ByteRange::ByteRange() : kind_(NONE) , first_(0) - , last_(-1) + , last_(INT64_MAX) {} ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 43259d8dc..931069b10 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -65,31 +65,23 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) ByteRange parse_byte_range(std::string range) { - const int64_t int64_max = std::numeric_limits::max(); - ByteRange byteRange; const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(range, byteUnitSpec) ) { range.erase(0, byteUnitSpec.size()); - int start = 0; - int end = -1; std::istringstream iss(range); - char c; - iss >> start; - if ( start < 0 ) { - if ( iss.eof() ) - byteRange = ByteRange(ByteRange::PARSED, start, int64_max); - } else { - iss >> c; - if (iss.good() && c=='-') { - iss >> end; - if (iss.fail()) { - // Something went wrong while extracting - end = -1; - } - if (iss.eof()) { + int64_t start, end = INT64_MAX; + if (iss >> start) { + if ( start < 0 ) { + if ( iss.eof() ) byteRange = ByteRange(ByteRange::PARSED, start, end); + } else { + char c; + if (iss >> c && c=='-') { + iss >> end; // if this fails, end is not modified, which is OK + if (iss.eof()) + byteRange = ByteRange(ByteRange::PARSED, start, end); } } } diff --git a/src/server/response.cpp b/src/server/response.cpp index 77d545a53..3e721b3ae 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -50,9 +50,7 @@ ByteRange resolve_byte_range(const kiwix::Entry& entry, ByteRange range) ? std::max(int64_t(0), entrySize + range.first()) : range.first(); - const int64_t resolved_last = range.last() < 0 - ? entrySize - 1 - : std::min(entrySize-1, range.last()); + const int64_t resolved_last = std::min(entrySize-1, range.last()); return ByteRange(ByteRange::RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); } From 67a347c0c47c253a80792c19c837ae7e1dc2f070 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 17:21:10 +0400 Subject: [PATCH 18/33] Moved byte-range parsing to byte_range.cpp --- src/server/byte_range.cpp | 28 ++++++++++++++++++++++++++++ src/server/byte_range.h | 3 +++ src/server/request_context.cpp | 29 +---------------------------- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index 5482914ab..14c2c33ea 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -20,6 +20,8 @@ #include "byte_range.h" +#include "tools/stringTools.h" + namespace kiwix { ByteRange::ByteRange() @@ -34,4 +36,30 @@ ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) , last_(last) {} +ByteRange ByteRange::parse(std::string rangeStr) +{ + ByteRange byteRange; + const std::string byteUnitSpec("bytes="); + if ( kiwix::startsWith(rangeStr, byteUnitSpec) ) { + rangeStr.erase(0, byteUnitSpec.size()); + std::istringstream iss(rangeStr); + + int64_t start, end = INT64_MAX; + if (iss >> start) { + if ( start < 0 ) { + if ( iss.eof() ) + byteRange = ByteRange(ByteRange::PARSED, start, end); + } else { + char c; + if (iss >> c && c=='-') { + iss >> end; // if this fails, end is not modified, which is OK + if (iss.eof()) + byteRange = ByteRange(ByteRange::PARSED, start, end); + } + } + } + } + return byteRange; +} + } // namespace kiwix diff --git a/src/server/byte_range.h b/src/server/byte_range.h index 4df6bd93c..7bdbbc090 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -22,6 +22,7 @@ #define KIWIXLIB_SERVER_BYTE_RANGE_H #include +#include namespace kiwix { @@ -51,6 +52,8 @@ class ByteRange int64_t last() const { return last_; } int64_t length() const { return last_ + 1 - first_; } + static ByteRange parse(std::string rangeStr); + private: // data Kind kind_; int64_t first_; diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 931069b10..55836f4f4 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -20,7 +20,6 @@ #include "request_context.h" -#include "tools/stringTools.h" #include #include #include @@ -63,32 +62,6 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) } } -ByteRange parse_byte_range(std::string range) -{ - ByteRange byteRange; - const std::string byteUnitSpec("bytes="); - if ( kiwix::startsWith(range, byteUnitSpec) ) { - range.erase(0, byteUnitSpec.size()); - std::istringstream iss(range); - - int64_t start, end = INT64_MAX; - if (iss >> start) { - if ( start < 0 ) { - if ( iss.eof() ) - byteRange = ByteRange(ByteRange::PARSED, start, end); - } else { - char c; - if (iss >> c && c=='-') { - iss >> end; // if this fails, end is not modified, which is OK - if (iss.eof()) - byteRange = ByteRange(ByteRange::PARSED, start, end); - } - } - } - } - return byteRange; -} - } // unnamed namespace @@ -114,7 +87,7 @@ RequestContext::RequestContext(struct MHD_Connection* connection, } catch (const std::out_of_range&) {} try { - byteRange_ = parse_byte_range(get_header(MHD_HTTP_HEADER_RANGE)); + byteRange_ = ByteRange::parse(get_header(MHD_HTTP_HEADER_RANGE)); } catch (const std::out_of_range&) {} } From 801ad18a891544a34bedf050f9be5c13ae550047 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 17:27:35 +0400 Subject: [PATCH 19/33] ByteRange::resolve() --- src/server/byte_range.cpp | 14 ++++++++++++++ src/server/byte_range.h | 1 + src/server/response.cpp | 17 +---------------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index 14c2c33ea..e00238af6 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -62,4 +62,18 @@ ByteRange ByteRange::parse(std::string rangeStr) return byteRange; } +ByteRange ByteRange::resolve(int64_t contentSize) const +{ + if ( kind() == NONE ) + return ByteRange(RESOLVED_FULL_CONTENT, 0, contentSize-1); + + const int64_t resolved_first = first() < 0 + ? std::max(int64_t(0), contentSize + first()) + : first(); + + const int64_t resolved_last = std::min(contentSize-1, last()); + + return ByteRange(RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); +} + } // namespace kiwix diff --git a/src/server/byte_range.h b/src/server/byte_range.h index 7bdbbc090..c2c15af7e 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -53,6 +53,7 @@ class ByteRange int64_t length() const { return last_ + 1 - first_; } static ByteRange parse(std::string rangeStr); + ByteRange resolve(int64_t contentSize) const; private: // data Kind kind_; diff --git a/src/server/response.cpp b/src/server/response.cpp index 3e721b3ae..7b1e8f803 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -39,21 +39,6 @@ bool is_compressible_mime_type(const std::string& mimeType) || mimeType.find("application/json") != string::npos; } -ByteRange resolve_byte_range(const kiwix::Entry& entry, ByteRange range) -{ - const int64_t entrySize = entry.getSize(); - - if ( range.kind() == ByteRange::NONE ) - return ByteRange(ByteRange::RESOLVED_FULL_CONTENT, 0, entrySize-1); - - const int64_t resolved_first = range.first() < 0 - ? std::max(int64_t(0), entrySize + range.first()) - : range.first(); - - const int64_t resolved_last = std::min(entrySize-1, range.last()); - - return ByteRange(ByteRange::RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); -} } // unnamed namespace @@ -338,7 +323,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_content(content); set_compress(true); } else { - m_byteRange = resolve_byte_range(entry, request.get_range()); + m_byteRange = request.get_range().resolve(entry.getSize()); } } From f7571b5b69f64cd861fb2bb900765d3d22f89fcd Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 25 May 2020 17:42:18 +0400 Subject: [PATCH 20/33] Content-Range header is set only for partial content --- src/server/response.cpp | 12 +++++++----- test/server.cpp | 7 +++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 7b1e8f803..b0811f63f 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -241,12 +241,14 @@ Response::create_entry_mhd_response() const MHD_add_response_header(response, MHD_HTTP_HEADER_CONTENT_TYPE, m_mimeType.c_str()); MHD_add_response_header(response, MHD_HTTP_HEADER_ACCEPT_RANGES, "bytes"); - std::ostringstream oss; - oss << "bytes " << m_byteRange.first() << "-" << m_byteRange.last() - << "/" << m_entry.getSize(); + if ( m_byteRange.kind() == ByteRange::RESOLVED_PARTIAL_CONTENT ) { + std::ostringstream oss; + oss << "bytes " << m_byteRange.first() << "-" << m_byteRange.last() + << "/" << m_entry.getSize(); - MHD_add_response_header(response, - MHD_HTTP_HEADER_CONTENT_RANGE, oss.str().c_str()); + MHD_add_response_header(response, + MHD_HTTP_HEADER_CONTENT_RANGE, oss.str().c_str()); + } MHD_add_response_header(response, MHD_HTTP_HEADER_CONTENT_LENGTH, kiwix::to_string(content_length).c_str()); diff --git a/test/server.cpp b/test/server.cpp index 6031b0e65..9d41ebb69 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -413,12 +413,15 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) { const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; const auto full = zfs1_->GET(url); + EXPECT_FALSE(full->has_header("Content-Range")); + EXPECT_EQ("bytes", full->get_header_value("Accept-Ranges")); { const auto p = zfs1_->GET(url, { {"Range", "bytes=0-100000"} } ); EXPECT_EQ(206, p->status); EXPECT_EQ(full->body, p->body); EXPECT_EQ("bytes 0-20076/20077", p->get_header_value("Content-Range")); + EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } { @@ -427,6 +430,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ("bytes 0-10/20077", p->get_header_value("Content-Range")); EXPECT_EQ(11, p->body.size()); EXPECT_EQ(full->body.substr(0, 11), p->body); + EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } { @@ -435,6 +439,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ("bytes 123-456/20077", p->get_header_value("Content-Range")); EXPECT_EQ(334, p->body.size()); EXPECT_EQ(full->body.substr(123, 334), p->body); + EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } { @@ -442,6 +447,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ(206, p->status); EXPECT_EQ(full->body.substr(20000), p->body); EXPECT_EQ("bytes 20000-20076/20077", p->get_header_value("Content-Range")); + EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } { @@ -449,5 +455,6 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ(206, p->status); EXPECT_EQ(full->body.substr(19977), p->body); EXPECT_EQ("bytes 19977-20076/20077", p->get_header_value("Content-Range")); + EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } } From 931e95f391ca994b454a5edb4ece6e41d30c8934 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 01:40:07 +0400 Subject: [PATCH 21/33] Invalid byte ranges result in 416 responses --- src/server/byte_range.cpp | 8 ++++++-- src/server/byte_range.h | 5 +++++ src/server/response.cpp | 42 +++++++++++++++++++++++++++++---------- src/server/response.h | 2 ++ test/server.cpp | 19 ++++++++++++++++++ 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index e00238af6..3102f0745 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -38,7 +38,7 @@ ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) ByteRange ByteRange::parse(std::string rangeStr) { - ByteRange byteRange; + ByteRange byteRange(INVALID, 0, INT64_MAX); const std::string byteUnitSpec("bytes="); if ( kiwix::startsWith(rangeStr, byteUnitSpec) ) { rangeStr.erase(0, byteUnitSpec.size()); @@ -53,12 +53,13 @@ ByteRange ByteRange::parse(std::string rangeStr) char c; if (iss >> c && c=='-') { iss >> end; // if this fails, end is not modified, which is OK - if (iss.eof()) + if (iss.eof() && start <= end) byteRange = ByteRange(ByteRange::PARSED, start, end); } } } } + return byteRange; } @@ -67,6 +68,9 @@ ByteRange ByteRange::resolve(int64_t contentSize) const if ( kind() == NONE ) return ByteRange(RESOLVED_FULL_CONTENT, 0, contentSize-1); + if ( kind() == INVALID ) + return ByteRange(INVALID, 0, contentSize-1); + const int64_t resolved_first = first() < 0 ? std::max(int64_t(0), contentSize + first()) : first(); diff --git a/src/server/byte_range.h b/src/server/byte_range.h index c2c15af7e..e302cece4 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -33,6 +33,11 @@ class ByteRange // No byte-range was present in the request NONE, + // The value of the Range header is not a valid continuous range. + // Note that a valid (according to RFC7233) sequence of byte ranges is + // considered invalid in this context. + INVALID, + // This byte-range has been parsed from request PARSED, diff --git a/src/server/response.cpp b/src/server/response.cpp index b0811f63f..aafe1336b 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -169,6 +169,20 @@ Response::can_compress(const RequestContext& request) const && (m_content.size() > KIWIX_MIN_CONTENT_SIZE_TO_DEFLATE); } +MHD_Response* +Response::create_error_response(const RequestContext& request) const +{ + MHD_Response* response = MHD_create_response_from_buffer(0, NULL, MHD_RESPMEM_PERSISTENT); + if ( m_returnCode == MHD_HTTP_RANGE_NOT_SATISFIABLE ) { + std::ostringstream oss; + oss << "bytes */" << m_byteRange.length(); + + MHD_add_response_header(response, + MHD_HTTP_HEADER_CONTENT_RANGE, oss.str().c_str()); + } + return response; +} + MHD_Response* Response::create_raw_content_mhd_response(const RequestContext& request) { @@ -259,6 +273,9 @@ MHD_Response* Response::create_mhd_response(const RequestContext& request) { switch (m_mode) { + case ResponseMode::ERROR: + return create_error_response(request); + case ResponseMode::RAW_CONTENT : return create_raw_content_mhd_response(request); @@ -275,12 +292,14 @@ int Response::send(const RequestContext& request, MHD_Connection* connection) { MHD_Response* response = create_mhd_response(request); - MHD_add_response_header(response, "Access-Control-Allow-Origin", "*"); - MHD_add_response_header(response, MHD_HTTP_HEADER_CACHE_CONTROL, - m_etag.get_option(ETag::CACHEABLE_ENTITY) ? "max-age=2723040, public" : "no-cache, no-store, must-revalidate"); - const std::string etag = m_etag.get_etag(); - if ( ! etag.empty() ) - MHD_add_response_header(response, MHD_HTTP_HEADER_ETAG, etag.c_str()); + if ( m_mode != ResponseMode::ERROR ) { + MHD_add_response_header(response, "Access-Control-Allow-Origin", "*"); + MHD_add_response_header(response, MHD_HTTP_HEADER_CACHE_CONTROL, + m_etag.get_option(ETag::CACHEABLE_ENTITY) ? "max-age=2723040, public" : "no-cache, no-store, must-revalidate"); + const std::string etag = m_etag.get_etag(); + if ( ! etag.empty() ) + MHD_add_response_header(response, MHD_HTTP_HEADER_ETAG, etag.c_str()); + } if (m_returnCode == MHD_HTTP_OK && m_byteRange.kind() == ByteRange::RESOLVED_PARTIAL_CONTENT) m_returnCode = MHD_HTTP_PARTIAL_CONTENT; @@ -316,16 +335,17 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_mimeType(mimeType); set_cacheable(); - // XXX: Which of the Accept-Encoding and Range headers has precedence if both - // XXX: are present? Can partial content be compressed? - if ( is_compressible_mime_type(mimeType) ) { + m_byteRange = request.get_range().resolve(entry.getSize()); + if ( m_byteRange.kind() == ByteRange::RESOLVED_FULL_CONTENT && is_compressible_mime_type(mimeType) ) { zim::Blob raw_content = entry.getBlob(); const std::string content = string(raw_content.data(), raw_content.size()); set_content(content); set_compress(true); - } else { - m_byteRange = request.get_range().resolve(entry.getSize()); + } else if ( m_byteRange.kind() == ByteRange::INVALID ) { + set_code(MHD_HTTP_RANGE_NOT_SATISFIABLE); + set_content(""); + m_mode = ResponseMode::ERROR; } } diff --git a/src/server/response.h b/src/server/response.h index 9fba5a191..9ef0d9486 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -35,6 +35,7 @@ extern "C" { namespace kiwix { enum class ResponseMode { + ERROR, RAW_CONTENT, REDIRECTION, ENTRY @@ -73,6 +74,7 @@ class Response { private: // functions MHD_Response* create_mhd_response(const RequestContext& request); + MHD_Response* create_error_response(const RequestContext& request) const; MHD_Response* create_raw_content_mhd_response(const RequestContext& request); MHD_Response* create_redirection_mhd_response() const; MHD_Response* create_entry_mhd_response() const; diff --git a/test/server.cpp b/test/server.cpp index 9d41ebb69..22a60490a 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -458,3 +458,22 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges")); } } + +TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) +{ + const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + + const char* invalidRanges[] = { + "0-10", "bytes=", "bytes=123", "bytes=-10-20", "bytes=10-20xxx", + "bytes=10-0", "bytes=10-20, 30-40" + }; + + for( const char* range : invalidRanges ) + { + const TestContext ctx{ {"Range", range} }; + const auto p = zfs1_->GET(url, { {"Range", range } } ); + EXPECT_EQ(416, p->status) << ctx; + EXPECT_TRUE(p->body.empty()) << ctx; + EXPECT_EQ("bytes */20077", p->get_header_value("Content-Range")) << ctx; + } +} From ff23b28e7c82f74b39ee44fc1950aeed6f1f15a5 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 01:41:37 +0400 Subject: [PATCH 22/33] Removed unnecessary qualifier --- src/server/byte_range.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index 3102f0745..dd21da8a6 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -48,13 +48,13 @@ ByteRange ByteRange::parse(std::string rangeStr) if (iss >> start) { if ( start < 0 ) { if ( iss.eof() ) - byteRange = ByteRange(ByteRange::PARSED, start, end); + byteRange = ByteRange(PARSED, start, end); } else { char c; if (iss >> c && c=='-') { iss >> end; // if this fails, end is not modified, which is OK if (iss.eof() && start <= end) - byteRange = ByteRange(ByteRange::PARSED, start, end); + byteRange = ByteRange(PARSED, start, end); } } } From 7301bf89bb4b72ed34b4e27474a6e10705b3b059 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 01:50:29 +0400 Subject: [PATCH 23/33] Some refactoring of byte-range parsing --- src/server/byte_range.cpp | 52 ++++++++++++++++++++++----------------- src/server/byte_range.h | 2 +- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index dd21da8a6..45f10b668 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -24,6 +24,32 @@ namespace kiwix { +namespace { + +ByteRange parseByteRange(const std::string& rangeStr) +{ + std::istringstream iss(rangeStr); + + int64_t start, end = INT64_MAX; + if (iss >> start) { + if ( start < 0 ) { + if ( iss.eof() ) + return ByteRange(ByteRange::PARSED, start, end); + } else { + char c; + if (iss >> c && c=='-') { + iss >> end; // if this fails, end is not modified, which is OK + if (iss.eof() && start <= end) + return ByteRange(ByteRange::PARSED, start, end); + } + } + } + + return ByteRange(ByteRange::INVALID, 0, INT64_MAX); +} + +} // unnamed namespace + ByteRange::ByteRange() : kind_(NONE) , first_(0) @@ -36,31 +62,13 @@ ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) , last_(last) {} -ByteRange ByteRange::parse(std::string rangeStr) +ByteRange ByteRange::parse(const std::string& rangeStr) { - ByteRange byteRange(INVALID, 0, INT64_MAX); const std::string byteUnitSpec("bytes="); - if ( kiwix::startsWith(rangeStr, byteUnitSpec) ) { - rangeStr.erase(0, byteUnitSpec.size()); - std::istringstream iss(rangeStr); + if ( ! kiwix::startsWith(rangeStr, byteUnitSpec) ) + return ByteRange(INVALID, 0, INT64_MAX); - int64_t start, end = INT64_MAX; - if (iss >> start) { - if ( start < 0 ) { - if ( iss.eof() ) - byteRange = ByteRange(PARSED, start, end); - } else { - char c; - if (iss >> c && c=='-') { - iss >> end; // if this fails, end is not modified, which is OK - if (iss.eof() && start <= end) - byteRange = ByteRange(PARSED, start, end); - } - } - } - } - - return byteRange; + return parseByteRange(rangeStr.substr(byteUnitSpec.size())); } ByteRange ByteRange::resolve(int64_t contentSize) const diff --git a/src/server/byte_range.h b/src/server/byte_range.h index e302cece4..848fd8082 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -57,7 +57,7 @@ class ByteRange int64_t last() const { return last_; } int64_t length() const { return last_ + 1 - first_; } - static ByteRange parse(std::string rangeStr); + static ByteRange parse(const std::string& rangeStr); ByteRange resolve(int64_t contentSize) const; private: // data From 6b43438b743342d1d7d5ce38ce68140b34fcde4a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 01:54:36 +0400 Subject: [PATCH 24/33] Fixed compilation error under native_dyn MHD_HTTP_RANGE_NOT_SATISFIABLE is not defined in the older version of libmicrohttpd (that is used under CI/Linux native_dyn). --- src/server/response.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index aafe1336b..d2a84f9c9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -173,7 +173,7 @@ MHD_Response* Response::create_error_response(const RequestContext& request) const { MHD_Response* response = MHD_create_response_from_buffer(0, NULL, MHD_RESPMEM_PERSISTENT); - if ( m_returnCode == MHD_HTTP_RANGE_NOT_SATISFIABLE ) { + if ( m_returnCode == 416 ) { std::ostringstream oss; oss << "bytes */" << m_byteRange.length(); @@ -343,7 +343,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_content(content); set_compress(true); } else if ( m_byteRange.kind() == ByteRange::INVALID ) { - set_code(MHD_HTTP_RANGE_NOT_SATISFIABLE); + set_code(416); set_content(""); m_mode = ResponseMode::ERROR; } From 37032892a46aeb291316e7934aa6b8d3f10a4e2f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 01:58:17 +0400 Subject: [PATCH 25/33] Fixed compilation error under win32_* ERROR is a macro under Windows --- src/server/response.cpp | 6 +++--- src/server/response.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index d2a84f9c9..dddc64da9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -273,7 +273,7 @@ MHD_Response* Response::create_mhd_response(const RequestContext& request) { switch (m_mode) { - case ResponseMode::ERROR: + case ResponseMode::ERROR_RESPONSE: return create_error_response(request); case ResponseMode::RAW_CONTENT : @@ -292,7 +292,7 @@ int Response::send(const RequestContext& request, MHD_Connection* connection) { MHD_Response* response = create_mhd_response(request); - if ( m_mode != ResponseMode::ERROR ) { + if ( m_mode != ResponseMode::ERROR_RESPONSE ) { MHD_add_response_header(response, "Access-Control-Allow-Origin", "*"); MHD_add_response_header(response, MHD_HTTP_HEADER_CACHE_CONTROL, m_etag.get_option(ETag::CACHEABLE_ENTITY) ? "max-age=2723040, public" : "no-cache, no-store, must-revalidate"); @@ -345,7 +345,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { } else if ( m_byteRange.kind() == ByteRange::INVALID ) { set_code(416); set_content(""); - m_mode = ResponseMode::ERROR; + m_mode = ResponseMode::ERROR_RESPONSE; } } diff --git a/src/server/response.h b/src/server/response.h index 9ef0d9486..49e346c02 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -35,7 +35,7 @@ extern "C" { namespace kiwix { enum class ResponseMode { - ERROR, + ERROR_RESPONSE, RAW_CONTENT, REDIRECTION, ENTRY From c2ebdefe8d7ccc32b8237845a36f779f6634d328 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 02:11:26 +0400 Subject: [PATCH 26/33] Handling of unsatisfiable ranges --- src/server/byte_range.cpp | 3 +++ test/server.cpp | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index 45f10b668..be1ddcb97 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -85,6 +85,9 @@ ByteRange ByteRange::resolve(int64_t contentSize) const const int64_t resolved_last = std::min(contentSize-1, last()); + if ( resolved_first > resolved_last ) + return ByteRange(INVALID, 0, contentSize-1); + return ByteRange(RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); } diff --git a/test/server.cpp b/test/server.cpp index 22a60490a..83037b7ff 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -465,7 +465,9 @@ TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) const char* invalidRanges[] = { "0-10", "bytes=", "bytes=123", "bytes=-10-20", "bytes=10-20xxx", - "bytes=10-0", "bytes=10-20, 30-40" + "bytes=10-0", // reversed range + "bytes=10-20, 30-40", // multi-range + "bytes=1000000-", "bytes=30000-30100" // unsatisfiable ranges }; for( const char* range : invalidRanges ) From 16bd79fa1b7a1da7ce89ba13ce29c34173c1725f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 12:50:08 +0400 Subject: [PATCH 27/33] Final clean-up of byte_range.{h,cpp} --- src/server/byte_range.cpp | 48 ++++++++++++++++++++++++++++++++------- src/server/byte_range.h | 38 ++++++++++++++++++++++--------- src/server/response.cpp | 2 +- 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/server/byte_range.cpp b/src/server/byte_range.cpp index be1ddcb97..e43faef1d 100644 --- a/src/server/byte_range.cpp +++ b/src/server/byte_range.cpp @@ -22,6 +22,8 @@ #include "tools/stringTools.h" +#include + namespace kiwix { namespace { @@ -34,7 +36,7 @@ ByteRange parseByteRange(const std::string& rangeStr) if (iss >> start) { if ( start < 0 ) { if ( iss.eof() ) - return ByteRange(ByteRange::PARSED, start, end); + return ByteRange(-start); } else { char c; if (iss >> c && c=='-') { @@ -60,7 +62,37 @@ ByteRange::ByteRange(Kind kind, int64_t first, int64_t last) : kind_(kind) , first_(first) , last_(last) -{} +{ + assert(kind != NONE); + assert(first >= 0); + assert(last >= first); +} + +ByteRange::ByteRange(int64_t suffix_length) + : kind_(PARSED) + , first_(-suffix_length) + , last_(INT64_MAX) +{ + assert(suffix_length > 0); +} + +int64_t ByteRange::first() const +{ + assert(kind_ > PARSED); + return first_; +} + +int64_t ByteRange::last() const +{ + assert(kind_ > PARSED); + return last_; +} + +int64_t ByteRange::length() const +{ + assert(kind_ > PARSED); + return last_ + 1 - first_; +} ByteRange ByteRange::parse(const std::string& rangeStr) { @@ -77,16 +109,16 @@ ByteRange ByteRange::resolve(int64_t contentSize) const return ByteRange(RESOLVED_FULL_CONTENT, 0, contentSize-1); if ( kind() == INVALID ) - return ByteRange(INVALID, 0, contentSize-1); + return ByteRange(RESOLVED_UNSATISFIABLE, 0, contentSize-1); - const int64_t resolved_first = first() < 0 - ? std::max(int64_t(0), contentSize + first()) - : first(); + const int64_t resolved_first = first_ < 0 + ? std::max(int64_t(0), contentSize + first_) + : first_; - const int64_t resolved_last = std::min(contentSize-1, last()); + const int64_t resolved_last = std::min(contentSize-1, last_); if ( resolved_first > resolved_last ) - return ByteRange(INVALID, 0, contentSize-1); + return ByteRange(RESOLVED_UNSATISFIABLE, 0, contentSize-1); return ByteRange(RESOLVED_PARTIAL_CONTENT, resolved_first, resolved_last); } diff --git a/src/server/byte_range.h b/src/server/byte_range.h index 848fd8082..2d1643f5b 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -29,33 +29,49 @@ namespace kiwix { class ByteRange { public: // types + + // ByteRange is parsed in a request, then it must be resolved (taking + // into account the actual size of the requested resource) before + // being applied in the response. + // The Kind enum represents possible states in such a lifecycle. enum Kind { - // No byte-range was present in the request + // The request is not a range request (no Range header) NONE, - // The value of the Range header is not a valid continuous range. - // Note that a valid (according to RFC7233) sequence of byte ranges is - // considered invalid in this context. + // The value of the Range header is not a valid continuous + // range. Note that a valid (according to RFC7233) sequence of multiple + // byte ranges is considered invalid in the current implementation + // (i.e. only single-range partial requests are supported). INVALID, - // This byte-range has been parsed from request + // This byte-range has been successfully parsed from the request PARSED, - // This is a response to a regular request + // This is a response to a regular (non-range) request RESOLVED_FULL_CONTENT, - // This is a response to a range request - RESOLVED_PARTIAL_CONTENT + // The range request is valid but unsatisfiable + RESOLVED_UNSATISFIABLE, + + // This is a response to a (satisfiable) range request + RESOLVED_PARTIAL_CONTENT, }; public: // functions + // Constructs a ByteRange object of NONE kind ByteRange(); + + // Constructs a ByteRange object of the given kind (except NONE) ByteRange(Kind kind, int64_t first, int64_t last); + // Constructs a ByteRange object of PARSED kind corresponding to a + // range request of the form "Range: bytes=-suffix_length" + explicit ByteRange(int64_t suffix_length); + Kind kind() const { return kind_; } - int64_t first() const { return first_; } - int64_t last() const { return last_; } - int64_t length() const { return last_ + 1 - first_; } + int64_t first() const; + int64_t last() const; + int64_t length() const; static ByteRange parse(const std::string& rangeStr); ByteRange resolve(int64_t contentSize) const; diff --git a/src/server/response.cpp b/src/server/response.cpp index dddc64da9..47050b4cd 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -342,7 +342,7 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_content(content); set_compress(true); - } else if ( m_byteRange.kind() == ByteRange::INVALID ) { + } else if ( m_byteRange.kind() == ByteRange::RESOLVED_UNSATISFIABLE ) { set_code(416); set_content(""); m_mode = ResponseMode::ERROR_RESPONSE; From 5f1918d005a7981374897b689f590304d354013c Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 13:04:03 +0400 Subject: [PATCH 28/33] Split a long line --- src/server/response.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 47050b4cd..2f76ed15e 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -336,7 +336,8 @@ void Response::set_entry(const Entry& entry, const RequestContext& request) { set_cacheable(); m_byteRange = request.get_range().resolve(entry.getSize()); - if ( m_byteRange.kind() == ByteRange::RESOLVED_FULL_CONTENT && is_compressible_mime_type(mimeType) ) { + const bool noRange = m_byteRange.kind() == ByteRange::RESOLVED_FULL_CONTENT; + if ( noRange && is_compressible_mime_type(mimeType) ) { zim::Blob raw_content = entry.getBlob(); const std::string content = string(raw_content.data(), raw_content.size()); From 85d6daabac9754330050ecde690c6303822da1e3 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 13:10:50 +0400 Subject: [PATCH 29/33] Rolled back minor unneeded changes --- src/server/request_context.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 55836f4f4..04ece7c5e 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -25,7 +25,6 @@ #include #include #include -#include namespace kiwix { @@ -62,7 +61,6 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation) } } - } // unnamed namespace RequestContext::RequestContext(struct MHD_Connection* connection, From a9b6d481cc3de11a49a30917bd09183f3a1b4a25 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 13:58:20 +0400 Subject: [PATCH 30/33] ServerTest.RangeHasPrecedenceOverCompression --- test/server.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/server.cpp b/test/server.cpp index 83037b7ff..16718b5dc 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -479,3 +479,18 @@ TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) EXPECT_EQ("bytes */20077", p->get_header_value("Content-Range")) << ctx; } } + +TEST_F(ServerTest, RangeHasPrecedenceOverCompression) +{ + const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + + const Headers onlyRange{ {"Range", "bytes=123-456"} }; + Headers rangeAndCompression(onlyRange); + rangeAndCompression.insert({"Accept-Encoding", "deflate"}); + + const auto p1 = zfs1_->GET(url, onlyRange); + const auto p2 = zfs1_->GET(url, rangeAndCompression); + EXPECT_EQ(p1->status, p2->status); + EXPECT_EQ(invariantHeaders(p1->headers), invariantHeaders(p2->headers)); + EXPECT_EQ(p1->body, p2->body); +} From 886ae1727439c4d83f6acd0c9759b1d359fdbf34 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 13:59:47 +0400 Subject: [PATCH 31/33] Fixed a CodeFactor issue --- src/server/byte_range.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server/byte_range.h b/src/server/byte_range.h index 2d1643f5b..c107eb5a1 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -29,7 +29,6 @@ namespace kiwix { class ByteRange { public: // types - // ByteRange is parsed in a request, then it must be resolved (taking // into account the actual size of the requested resource) before // being applied in the response. From 50a850f3a967ca2509f1b266e8f651aa158401b8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 14:04:18 +0400 Subject: [PATCH 32/33] Fixed a comment --- src/server/byte_range.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/byte_range.h b/src/server/byte_range.h index c107eb5a1..bba0ccb2f 100644 --- a/src/server/byte_range.h +++ b/src/server/byte_range.h @@ -49,7 +49,7 @@ class ByteRange // This is a response to a regular (non-range) request RESOLVED_FULL_CONTENT, - // The range request is valid but unsatisfiable + // The range request is invalid or unsatisfiable RESOLVED_UNSATISFIABLE, // This is a response to a (satisfiable) range request From f52b220d01e19777b4e2e777b5eb97af5abfec5a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 26 May 2020 14:10:26 +0400 Subject: [PATCH 33/33] Dropped RequestContext::has_range() --- src/server/request_context.cpp | 6 +----- src/server/request_context.h | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 04ece7c5e..9ed28be84 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -126,7 +126,7 @@ void RequestContext::print_debug_info() const { printf("full_url: %s\n", full_url.c_str()); printf("url : %s\n", url.c_str()); printf("acceptEncodingDeflate : %d\n", acceptEncodingDeflate); - printf("has_range : %d\n", has_range()); + printf("has_range : %d\n", byteRange_.kind() != ByteRange::NONE); printf("is_valid_url : %d\n", is_valid_url()); printf(".............\n"); } @@ -168,10 +168,6 @@ bool RequestContext::is_valid_url() const { return !url.empty(); } -bool RequestContext::has_range() const { - return byteRange_.kind() == ByteRange::PARSED; -} - ByteRange RequestContext::get_range() const { return byteRange_; } diff --git a/src/server/request_context.h b/src/server/request_context.h index 3256e30e5..4860fcf6e 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -80,7 +80,6 @@ class RequestContext { std::string get_url_part(int part) const; std::string get_full_url() const; - bool has_range() const; ByteRange get_range() const; bool can_compress() const { return acceptEncodingDeflate; }