aseba-community / aseba

Aseba is a set of tools which allow beginners to program robots easily and efficiently. To contact us, please open an issue.
http://aseba.wikidot.com
GNU Lesser General Public License v3.0
49 stars 61 forks source link

No error page in asebahttp2 #643

Open ypiguet-epfl opened 7 years ago

ypiguet-epfl commented 7 years ago

When a request to asebahttp2 fails with page not found, the correct http status code 404 is sent to the client, but the content is empty with content-type application/json (empty is invalid for json). While Chrome displays an error message, Firefox shows an empty page.

I suggest to reply with visible content and a valid content-type for it. Here is a simple patch. The setErrorReply method should also be called instead of setStatus from other places.

diff --git a/switches/http2/HttpHandler.h b/switches/http2/HttpHandler.h
index 69dbc4c..d90d2a5 100644
--- a/switches/http2/HttpHandler.h
+++ b/switches/http2/HttpHandler.h
@@ -98,7 +98,7 @@ namespace Aseba { namespace Http
                                        }
                                }

-                               request->respond().setStatus(HttpResponse::HTTP_STATUS_NOT_FOUND);
+                               request->respond().setErrorReply(HttpResponse::HTTP_STATUS_NOT_FOUND);
                        }

                        virtual void addSubhandler(HttpHandler *subhandler) { subhandlers.push_back(subhandler); }setErrorReply
diff --git a/switches/http2/HttpResponse.h b/switches/http2/HttpResponse.h
index cde9045..1b5d7eb 100644
--- a/switches/http2/HttpResponse.h
+++ b/switches/http2/HttpResponse.h
@@ -77,6 +77,14 @@ namespace Aseba { namespace Http
                                }
                        }

+                       virtual void setErrorReply(HttpStatus status) {
+                               setStatus(status);
+                               setHeader("Content-Type", "text/plain");
+                               std::stringstream content;
+                               content << "Error " << status << ".\n";
+                               setContent(content.str());
+                       }
+
                protected:
                        virtual void addStatusReply(std::ostringstream& reply);
                        virtual void addHeadersReply(std::ostringstream& reply);
stephanemagnenat commented 7 years ago

Note that the original http2 is unmaintained, there is an ongoing refactoring in the https://github.com/aseba-community/aseba/tree/http-refactor branch.

stephanemagnenat commented 7 years ago

But of course, if a small patch corrects a problem, we can merge it for sure.