Skip to content

Commit

Permalink
Server sets userlang cookie as global and permanent
Browse files Browse the repository at this point in the history
Without specifying the "Path" attribute of the cookie in the "Set-Cookie" header
we end up with multiple instances of the cookie for different URLs. We
want a single "global" cookie for kiwix-serve. Besides we want it to be
"permanent" rather than a session cookie, hence the large (1-year-long)
TTL value for the "Max-Age" attribute.
  • Loading branch information
veloman-yunkan authored and mgautierfr committed Jan 24, 2023
1 parent fcb97c3 commit e35e758
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 12 deletions.
9 changes: 7 additions & 2 deletions src/server/request_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,13 @@ fullURL2LocalURL(const std::string& full_url, const std::string& rootLocation)
} // unnamed namespace

RequestContext::RequestContext(struct MHD_Connection* connection,
std::string rootLocation,
std::string _rootLocation,
const std::string& _url,
const std::string& _method,
const std::string& version) :
rootLocation(_rootLocation),
full_url(_url),
url(fullURL2LocalURL(_url, rootLocation)),
url(fullURL2LocalURL(_url, _rootLocation)),
method(str2RequestMethod(_method)),
version(version),
requestIndex(s_requestIndex++),
Expand Down Expand Up @@ -193,6 +194,10 @@ std::string RequestContext::get_full_url() const {
return full_url;
}

std::string RequestContext::get_root_path() const {
return rootLocation.empty() ? "/" : rootLocation;
}

bool RequestContext::is_valid_url() const {
return !url.empty();
}
Expand Down
2 changes: 2 additions & 0 deletions src/server/request_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class RequestContext {
std::string get_url() const;
std::string get_url_part(int part) const;
std::string get_full_url() const;
std::string get_root_path() const;

std::string get_query() const { return queryString; }

Expand Down Expand Up @@ -136,6 +137,7 @@ class RequestContext {
};

private: // data
std::string rootLocation;
std::string full_url;
std::string url;
RequestMethod method;
Expand Down
4 changes: 3 additions & 1 deletion src/server/response.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,9 @@ MHD_Result Response::send(const RequestContext& request, MHD_Connection* connect
}

if ( ! request.user_language_comes_from_cookie() ) {
const std::string cookie = "userlang=" + request.get_user_language();
const std::string cookie = "userlang=" + request.get_user_language()
+ ";Path=" + request.get_root_path()
+ ";Max-Age=31536000";
MHD_add_response_header(response, MHD_HTTP_HEADER_SET_COOKIE, cookie.c_str());
}

Expand Down
18 changes: 9 additions & 9 deletions test/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1007,39 +1007,39 @@ TEST_F(ServerTest, UserLanguageControl)
/*url*/ "/ROOT/content/zimfile/invalid-article",
/*Accept-Language:*/ "",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
{
"userlang URL query parameter is respected",
/*url*/ "/ROOT/content/zimfile/invalid-article?userlang=en",
/*Accept-Language:*/ "",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
{
"userlang URL query parameter is respected",
/*url*/ "/ROOT/content/zimfile/invalid-article?userlang=test",
/*Accept-Language:*/ "",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=test",
/*Response Set-Cookie:*/ "userlang=test;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "[I18N TESTING] Content not found, but at least the server is alive"
},
{
"'Accept-Language: *' is handled",
/*url*/ "/ROOT/content/zimfile/invalid-article",
/*Accept-Language:*/ "*",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
{
"Accept-Language: header is respected",
/*url*/ "/ROOT/content/zimfile/invalid-article",
/*Accept-Language:*/ "test",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=test",
/*Response Set-Cookie:*/ "userlang=test;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "[I18N TESTING] Content not found, but at least the server is alive"
},
{
Expand Down Expand Up @@ -1087,15 +1087,15 @@ TEST_F(ServerTest, UserLanguageControl)
/*url*/ "/ROOT/content/zimfile/invalid-article?userlang=en",
/*Accept-Language:*/ "test",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
{
"userlang query parameter takes precedence over its cookie counterpart",
/*url*/ "/ROOT/content/zimfile/invalid-article?userlang=en",
/*Accept-Language:*/ "",
/*Request Cookie:*/ "userlang=test",
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
{
Expand All @@ -1113,7 +1113,7 @@ TEST_F(ServerTest, UserLanguageControl)
/*url*/ "/ROOT/content/zimfile/invalid-article",
/*Accept-Language:*/ "test;q=0.9, en;q=0.2",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=test",
/*Response Set-Cookie:*/ "userlang=test;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "[I18N TESTING] Content not found, but at least the server is alive"
},
{
Expand All @@ -1123,7 +1123,7 @@ TEST_F(ServerTest, UserLanguageControl)
/*url*/ "/ROOT/content/zimfile/invalid-article",
/*Accept-Language:*/ "test;q=0.2, en;q=0.9",
/*Request Cookie:*/ NO_COOKIE,
/*Response Set-Cookie:*/ "userlang=en",
/*Response Set-Cookie:*/ "userlang=en;Path=/ROOT;Max-Age=31536000",
/* expected <h1> */ "Not Found"
},
};
Expand Down

0 comments on commit e35e758

Please sign in to comment.