From 0c01ec5bb5d271bad990fbbbd380fe8c3df393b7 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 09:36:00 +0100 Subject: [PATCH 1/6] Cleanup a bit kiwix-serve.cpp - Remove spaces at end of lines - Remove unused variables. --- src/server/kiwix-serve.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index 74dcb90..9d241d4 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -120,12 +120,12 @@ static std::string getMimeTypeForFile(const std::string& filename) { void introduceTaskbar(string &content, const string &humanReadableBookId) { if (!nosearchbarFlag) { - content = appendToFirstOccurence(content, "", + content = appendToFirstOccurence(content, "", replaceRegex(RESOURCE::include_html_part, humanReadableBookId, "__CONTENT__")); content = appendToFirstOccurence(content, "", ""); - content = appendToFirstOccurence(content, "]*>", + content = appendToFirstOccurence(content, "]*>", replaceRegex(RESOURCE::taskbar_html_part, humanReadableBookId, "__CONTENT__")); } @@ -426,7 +426,6 @@ struct MHD_Response* handle_random(struct MHD_Connection * connection, bool acceptEncodingDeflate) { std::string httpRedirection; - bool cacheEnabled = false; httpResponseCode = MHD_HTTP_FOUND; if (reader != NULL) { pthread_mutex_lock(&readerLock); @@ -449,7 +448,6 @@ struct MHD_Response* handle_content(struct MHD_Connection * connection, std::string baseUrl; std::string content; std::string mimeType; - unsigned int contentLength; bool found = false; zim::Article article; @@ -591,7 +589,7 @@ static int accessHandlerCallback(void *cls, const char* acceptEncodingHeaderValue = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, MHD_HTTP_HEADER_ACCEPT_ENCODING); const bool acceptEncodingDeflate = acceptEncodingHeaderValue && string(acceptEncodingHeaderValue).find("deflate") != string::npos; - /* Check if range is requested */ + /* Check if range is requested. [TODO] Handle this somehow */ const char* acceptRangeHeaderValue = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, MHD_HTTP_HEADER_RANGE); const bool acceptRange = acceptRangeHeaderValue != NULL; @@ -607,7 +605,7 @@ static int accessHandlerCallback(void *cls, const char* tmpGetValue = MHD_lookup_connection_value(connection, MHD_GET_ARGUMENT_KIND, "content"); humanReadableBookId = (tmpGetValue != NULL ? string(tmpGetValue) : ""); } else { - humanReadableBookId = urlStr.substr(1, urlStr.find("/", 1) != string::npos ? + humanReadableBookId = urlStr.substr(1, urlStr.find("/", 1) != string::npos ? urlStr.find("/", 1) - 1 : urlStr.size() - 2); if (!humanReadableBookId.empty()) { urlStr = urlStr.substr(urlStr.find("/", 1) != string::npos ? @@ -697,7 +695,7 @@ static int accessHandlerCallback(void *cls, httpResponseCode, response); MHD_destroy_response(response); - + return ret; } @@ -792,7 +790,7 @@ int main(int argc, char **argv) { if (libraryFlag) { vector libraryPaths = kiwix::split(libraryPath, ";"); vector::iterator itr; - + for ( itr = libraryPaths.begin(); itr != libraryPaths.end(); ++itr ) { if (!itr->empty()) { bool retVal = false; @@ -924,7 +922,7 @@ int main(int argc, char **argv) { pthread_mutex_init(&compressorLock, NULL); pthread_mutex_init(&verboseFlagLock, NULL); pthread_mutex_init(&mimeTypeLock, NULL); - + /* Hard coded mimetypes */ extMimeTypes["html"] = "text/html"; extMimeTypes["htm"] = "text/html"; @@ -945,7 +943,7 @@ int main(int argc, char **argv) { extMimeTypes["ttf"] = "application/font-ttf"; extMimeTypes["woff"] = "application/font-woff"; extMimeTypes["vtt"] = "text/vtt"; - + /* Start the HTTP daemon */ void *page = NULL; if (interface.length() > 0) { @@ -956,7 +954,6 @@ int main(int argc, char **argv) { struct sockaddr_in sockAddr; struct ifaddrs *ifaddr, *ifa; int family, n; - char host[NI_MAXHOST]; /* Search all available interfaces */ if (getifaddrs(&ifaddr) == -1) { From a27010c247fb4e6207e2a36b07d336846753acff Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 09:43:26 +0100 Subject: [PATCH 2/6] Correctly change the compression buffer we send. For ie browser, we need to remove the two first bytes. If we make our buffer start two bytes after, we also need to reduce the size of the buffer by two bytes. Else we will read and send two extra junk bytes. Fix #15 --- src/server/kiwix-serve.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index 9d241d4..482e9fc 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -167,7 +167,7 @@ bool compress_content(string &content, comprLen = COMPRESSOR_BUFFER_SIZE; compress(compr, &comprLen, (const Bytef*)(content.data()), contentLength); - if (comprLen > 2 && comprLen < contentLength) { + if (comprLen > 2 && comprLen < (contentLength+2)) { /* /!\ Internet Explorer has a bug with deflate compression. It can not handle the first two bytes (compression headers) @@ -175,6 +175,7 @@ bool compress_content(string &content, It has no incidence on other browsers See http://www.subbu.org/blog/2008/03/ie7-deflate-or-not and comments */ compr += 2; + comprLen -= 2; content = string((char *)compr, comprLen); contentLength = comprLen; From 3592cd84c66b01c00c82e80de9aeca3ce7c65955 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 09:49:21 +0100 Subject: [PATCH 3/6] Do not modify the compr buffer pointer. The compr pointer points to the allocated memory. We must not modify it value. If we advance the pointer by two bytes each time we compress an answer we will end to write in some random memory and segfault. Now, we use a std::vector to correctly handle allocation (and deallocation!) of the memory. --- src/server/kiwix-serve.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index 482e9fc..e2fa03f 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -49,6 +49,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -142,14 +143,13 @@ bool isVerbose() { /* For compression */ #define COMPRESSOR_BUFFER_SIZE 10000000 -static Bytef *compr = (Bytef *)malloc(COMPRESSOR_BUFFER_SIZE); -static uLongf comprLen; - static bool compress_content(string &content, const string &mimeType) { + static std::vector compr_buffer; + /* Compute the lengh */ unsigned int contentLength = content.size(); @@ -164,8 +164,9 @@ bool compress_content(string &content, /* Compress the content if necessary */ if (deflated) { pthread_mutex_lock(&compressorLock); - comprLen = COMPRESSOR_BUFFER_SIZE; - compress(compr, &comprLen, (const Bytef*)(content.data()), contentLength); + compr_buffer.reserve(COMPRESSOR_BUFFER_SIZE); + uLongf comprLen = COMPRESSOR_BUFFER_SIZE; + compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); if (comprLen > 2 && comprLen < (contentLength+2)) { @@ -174,11 +175,8 @@ bool compress_content(string &content, We need to chunk them off (move the content 2bytes) It has no incidence on other browsers See http://www.subbu.org/blog/2008/03/ie7-deflate-or-not and comments */ - compr += 2; - comprLen -= 2; - content = string((char *)compr, comprLen); - contentLength = comprLen; + content = string((char *)&compr_buffer[2], comprLen-2); } else { deflated = false; } From e27ce444c61c4159dc2e8e7790b2f641c5cf0967 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 09:50:43 +0100 Subject: [PATCH 4/6] Correctly check that compress gone well before using the result buffer. We need to check the return code of compress. Compress may fail for different reason, one being that the compr_buffer is not large enough. --- src/server/kiwix-serve.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index e2fa03f..c5ae5c4 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -166,9 +166,9 @@ bool compress_content(string &content, pthread_mutex_lock(&compressorLock); compr_buffer.reserve(COMPRESSOR_BUFFER_SIZE); uLongf comprLen = COMPRESSOR_BUFFER_SIZE; - compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); + int err = compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); - if (comprLen > 2 && comprLen < (contentLength+2)) { + if (err == Z_OK && comprLen > 2 && comprLen < (contentLength+2)) { /* /!\ Internet Explorer has a bug with deflate compression. It can not handle the first two bytes (compression headers) From 063e9ba80d9e45b7fda87a38b2cc79a770f9ad86 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 09:54:34 +0100 Subject: [PATCH 5/6] Use compressBound function to reserve the right amount of space. compressBound function give the upper bound of space needed to reserve to the compression buffer. Use it instead of using a define. --- src/server/kiwix-serve.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index c5ae5c4..3e790d5 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -164,8 +164,9 @@ bool compress_content(string &content, /* Compress the content if necessary */ if (deflated) { pthread_mutex_lock(&compressorLock); - compr_buffer.reserve(COMPRESSOR_BUFFER_SIZE); - uLongf comprLen = COMPRESSOR_BUFFER_SIZE; + uLong bufferBound = compressBound(contentLength); + compr_buffer.reserve(bufferBound); + uLongf comprLen = compr_buffer.capacity(); int err = compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); if (err == Z_OK && comprLen > 2 && comprLen < (contentLength+2)) { From 35b83144c604d9fb39a1504ccf15c7d882bd071f Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 15 Mar 2017 10:00:08 +0100 Subject: [PATCH 6/6] Do not compress only if the content is small. We do not need the test "contentLength < COMPRESSOR_BUFFER_SIZE" to know if we compress the content or not. This is a non sens, we WANT to compress the content if it is big. --- src/server/kiwix-serve.cpp | 58 ++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/server/kiwix-serve.cpp b/src/server/kiwix-serve.cpp index 3e790d5..dba2ea5 100644 --- a/src/server/kiwix-serve.cpp +++ b/src/server/kiwix-serve.cpp @@ -141,49 +141,47 @@ bool isVerbose() { return value; } -/* For compression */ -#define COMPRESSOR_BUFFER_SIZE 10000000 - static bool compress_content(string &content, const string &mimeType) { static std::vector compr_buffer; + /* Should be deflate */ + bool deflated = mimeType.find("text/") != string::npos || + mimeType.find("application/javascript") != string::npos || + mimeType.find("application/json") != string::npos; + + if ( ! deflated ) + return false; + /* Compute the lengh */ unsigned int contentLength = content.size(); - /* Should be deflate */ - bool deflated = - contentLength > KIWIX_MIN_CONTENT_SIZE_TO_DEFLATE && - contentLength < COMPRESSOR_BUFFER_SIZE && - (mimeType.find("text/") != string::npos || - mimeType.find("application/javascript") != string::npos || - mimeType.find("application/json") != string::npos); + /* If the content is too short, no need to compress it */ + if ( contentLength <= KIWIX_MIN_CONTENT_SIZE_TO_DEFLATE) + return false; + + uLong bufferBound = compressBound(contentLength); /* Compress the content if necessary */ - if (deflated) { - pthread_mutex_lock(&compressorLock); - uLong bufferBound = compressBound(contentLength); - compr_buffer.reserve(bufferBound); - uLongf comprLen = compr_buffer.capacity(); - int err = compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); + pthread_mutex_lock(&compressorLock); + compr_buffer.reserve(bufferBound); + uLongf comprLen = compr_buffer.capacity(); + int err = compress(&compr_buffer[0], &comprLen, (const Bytef*)(content.data()), contentLength); - if (err == Z_OK && comprLen > 2 && comprLen < (contentLength+2)) { - - /* /!\ Internet Explorer has a bug with deflate compression. - It can not handle the first two bytes (compression headers) - We need to chunk them off (move the content 2bytes) - It has no incidence on other browsers - See http://www.subbu.org/blog/2008/03/ie7-deflate-or-not and comments */ - - content = string((char *)&compr_buffer[2], comprLen-2); - } else { - deflated = false; - } - - pthread_mutex_unlock(&compressorLock); + if (err == Z_OK && comprLen > 2 && comprLen < (contentLength+2)) { + /* /!\ Internet Explorer has a bug with deflate compression. + It can not handle the first two bytes (compression headers) + We need to chunk them off (move the content 2bytes) + It has no incidence on other browsers + See http://www.subbu.org/blog/2008/03/ie7-deflate-or-not and comments */ + content = string((char *)&compr_buffer[2], comprLen-2); + } else { + deflated = false; } + + pthread_mutex_unlock(&compressorLock); return deflated; }