smallsolar
/
libkiwix
mirror of https://github.com/kiwix/libkiwix.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #651 from kiwix/less_confusing_404_errors

This commit is contained in:
Matthieu Gautier 2021-12-22 17:30:29 +01:00 committed by GitHub
parent d21879ebda ed2f914e10
commit a77fae0993
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 45 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
src/server/internalServer.cpp
View File

@ -271,7 +271,7 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
{
try {
if (! request.is_valid_url())
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
const ETag etag = get_matching_if_none_match_etag(request);
if ( etag )
@ -385,24 +385,23 @@ SuggestionsList_t getSuggestions(const zim::Archive* const archive,
std::unique_ptr<Response> InternalServer::handle_meta(const RequestContext& request)
{
std::string bookName;
std::string bookId;
std::string meta_name;
std::shared_ptr<zim::Archive> archive;
try {
bookName = request.get_argument("content");
bookId = mp_nameMapper->getIdForName(bookName);
meta_name = request.get_argument("name");
const std::string bookId = mp_nameMapper->getIdForName(bookName);
archive = mp_library->getArchiveById(bookId);
} catch (const std::out_of_range& e) {
return Response::build_404(*this, request, bookName, "");
// error handled by the archive == nullptr check below
}
if (archive == nullptr) {
return Response::build_404(*this, request, bookName, "");
const std::string error_details = "No such book: " + bookName;
return Response::build_404(*this, "", bookName, "", error_details);
}
std::string content;
std::string mimeType = "text";
const auto meta_name = request.get_optional_param("name", std::string());
if (meta_name == "title") {
content = getArchiveTitle(*archive);
@ -425,7 +424,8 @@ std::unique_ptr<Response> InternalServer::handle_meta(const RequestContext& requ
} else if (const unsigned illustrationSize = parseIllustration(meta_name)) {
getArchiveFavicon(*archive, illustrationSize, content, mimeType);
} else {
return Response::build_404(*this, request, bookName, "");
const std::string error_details = "No such metadata item: " + meta_name;
return Response::build_404(*this, "", bookName, "", error_details);
}
auto response = ContentResponse::build(*this, content, mimeType);
@ -439,38 +439,26 @@ std::unique_ptr<Response> InternalServer::handle_suggest(const RequestContext& r
printf("** running handle_suggest\n");
}
std::string content;
std::string mimeType;
std::string bookName;
std::string bookId;
std::string queryString;
std::shared_ptr<zim::Archive> archive;
try {
bookName = request.get_argument("content");
bookId = mp_nameMapper->getIdForName(bookName);
queryString = request.get_argument("term");
const std::string bookId = mp_nameMapper->getIdForName(bookName);
archive = mp_library->getArchiveById(bookId);
} catch (const std::out_of_range&) {
return Response::build_404(*this, request, bookName, "");
}
auto start = 0;
try {
start = request.get_argument<unsigned int>("start");
} catch (const std::exception&) {}
unsigned int count = 10;
try {
count = request.get_argument<unsigned int>("count");
} catch (const std::exception&) {}
if (count == 0) {
count = 10;
// error handled by the archive == nullptr check below
}
if (archive == nullptr) {
return Response::build_404(*this, request, bookName, "");
const std::string error_details = "No such book: " + bookName;
return Response::build_404(*this, "", bookName, "", error_details);
}
const auto queryString = request.get_optional_param("term", std::string());
const auto start = request.get_optional_param<unsigned int>("start", 0);
unsigned int count = request.get_optional_param<unsigned int>("count", 10);
if (count == 0) {
count = 10;
}
if (m_verbose.load()) {
@ -532,7 +520,7 @@ std::unique_ptr<Response> InternalServer::handle_skin(const RequestContext& requ
response->set_cacheable();
return std::move(response);
} catch (const ResourceNotFound& e) {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
}
@ -542,13 +530,6 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
printf("** running handle_search\n");
}
std::string bookName;
std::string bookId;
try {
bookName = request.get_argument("content");
bookId = mp_nameMapper->getIdForName(bookName);
} catch (const std::out_of_range&) {}
std::string patternString;
try {
patternString = request.get_argument("pattern");
@ -567,8 +548,11 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
} catch(const std::out_of_range&) {}
catch(const std::invalid_argument&) {}
std::string bookName;
std::shared_ptr<zim::Archive> archive;
try {
bookName = request.get_argument("content");
const std::string bookId = mp_nameMapper->getIdForName(bookName);
archive = mp_library->getArchiveById(bookId);
} catch (const std::out_of_range&) {}
@ -661,25 +645,26 @@ std::unique_ptr<Response> InternalServer::handle_random(const RequestContext& re
}
std::string bookName;
std::string bookId;
std::shared_ptr<zim::Archive> archive;
try {
bookName = request.get_argument("content");
bookId = mp_nameMapper->getIdForName(bookName);
const std::string bookId = mp_nameMapper->getIdForName(bookName);
archive = mp_library->getArchiveById(bookId);
} catch (const std::out_of_range&) {
return Response::build_404(*this, request, bookName, "");
// error handled by the archive == nullptr check below
}
if (archive == nullptr) {
return Response::build_404(*this, request, bookName, "");
const std::string error_details = "No such book: " + bookName;
return Response::build_404(*this, "", bookName, "", error_details);
}
try {
auto entry = archive->getRandomEntry();
return build_redirect(bookName, getFinalItem(*archive, entry));
} catch(zim::EntryNotFound& e) {
return Response::build_404(*this, request, bookName, "");
const std::string error_details = "Oops! Failed to pick a random article :(";
return Response::build_404(*this, "", bookName, getArchiveTitle(*archive), error_details);
}
}
@ -691,7 +676,7 @@ std::unique_ptr<Response> InternalServer::handle_captured_external(const Request
} catch (const std::out_of_range& e) {}
if (source.empty())
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
auto data = get_default_data();
data.set("source", source);
@ -710,7 +695,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog(const RequestContext& r
host = request.get_header("Host");
url = request.get_url_part(1);
} catch (const std::out_of_range&) {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
if (url == "v2") {
@ -718,7 +703,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog(const RequestContext& r
}
if (url != "searchdescription.xml" && url != "root.xml" && url != "search") {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
if (url == "searchdescription.xml") {
@ -854,7 +839,7 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
std::string searchURL = m_root+"/search?pattern="+pattern; // Make a full search on the entire library.
const std::string details = searchSuggestionHTML(searchURL, kiwix::urlDecode(pattern));
return Response::build_404(*this, request, bookName, "", details);
return Response::build_404(*this, request.get_full_url(), bookName, "", details);
}
auto urlStr = request.get_url().substr(bookName.size()+1);
@ -887,7 +872,7 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
std::string searchURL = m_root+"/search?content="+bookName+"&pattern="+pattern; // Make a search on this specific book only.
const std::string details = searchSuggestionHTML(searchURL, kiwix::urlDecode(pattern));
return Response::build_404(*this, request, bookName, getArchiveTitle(*archive), details);
return Response::build_404(*this, request.get_full_url(), bookName, getArchiveTitle(*archive), details);
}
}

6
src/server/internalServer_catalog_v2.cpp
View File

@ -43,7 +43,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2(const RequestContext
try {
url = request.get_url_part(2);
} catch (const std::out_of_range&) {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
if (url == "root.xml") {
@ -67,7 +67,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2(const RequestContext
} else if (url == "languages") {
return handle_catalog_v2_languages(request);
} else {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
}
@ -108,7 +108,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_complete_entry(const
try {
mp_library->getBookById(entryId);
} catch (const std::out_of_range&) {
return Response::build_404(*this, request, "", "");
return Response::build_404(*this, request.get_full_url(), "", "");
}
OPDSDumper opdsDumper(mp_library);

6
src/server/response.cpp
View File

@ -83,10 +83,12 @@ std::unique_ptr<Response> Response::build_304(const InternalServer& server, cons
return response;
}
std::unique_ptr<Response> Response::build_404(const InternalServer& server, const RequestContext& request, const std::string& bookName, const std::string& bookTitle, const std::string& details)
std::unique_ptr<Response> Response::build_404(const InternalServer& server, const std::string& url, const std::string& bookName, const std::string& bookTitle, const std::string& details)
{
MustacheData results;
results.set("url", request.get_full_url());
if ( !url.empty() ) {
results.set("url", url);
}
results.set("details", details);
auto response = ContentResponse::build(server, RESOURCE::templates::_404_html, results, "text/html");

2
src/server/response.h
View File

@ -47,7 +47,7 @@ class Response {
static std::unique_ptr<Response> build(const InternalServer& server);
static std::unique_ptr<Response> build_304(const InternalServer& server, const ETag& etag);
static std::unique_ptr<Response> build_404(const InternalServer& server, const RequestContext& request, const std::string& bookName, const std::string& bookTitle, const std::string& details="");
static std::unique_ptr<Response> build_404(const InternalServer& server, const std::string& url, const std::string& bookName, const std::string& bookTitle, const std::string& details="");
static std::unique_ptr<Response> build_416(const InternalServer& server, size_t resourceLength);
static std::unique_ptr<Response> build_500(const InternalServer& server, const std::string& msg);
static std::unique_ptr<Response> build_redirect(const InternalServer& server, const std::string& redirectUrl);

2
static/templates/404.html
View File

@ -6,9 +6,11 @@
</head>
<body>
<h1>Not Found</h1>
{{#url}}
<p>
The requested URL "{{url}}" was not found on this server.
</p>
{{/url}}
{{#details}}
<p>
{{{details}}}

2
test/server.cpp
View File

@ -179,6 +179,7 @@ const ResourceCollection resources200Compressible{
{ NO_ETAG, "/search?content=zimfile&pattern=a" },
{ NO_ETAG, "/suggest?content=zimfile" },
{ NO_ETAG, "/suggest?content=zimfile&term=ray" },
{ NO_ETAG, "/catch/external?source=www.example.com" },
@ -282,7 +283,6 @@ const char* urls404[] = {
"/random?content=non-existent-book",
"/search",
"/suggest",
"/suggest?content=zimfile",
"/suggest?content=non-existent-book&term=abcd",
"/catch/external",
"/zimfile/A/non-existent-article",