From b574b5776695eb30e034fd8c7a468b3f03d4c6b9 Mon Sep 17 00:00:00 2001 From: Elad Nachmias Date: Tue, 27 Feb 2018 14:32:38 +0100 Subject: [PATCH] swarm: give correct error on 0x hash prefix (#16195) - added a case error struct that contains information about certain error cases in which we would like to output more information to the client - added a validation method that iterates and adds the information that is stored in the error cases --- .gitignore | 3 +++ swarm/api/http/error.go | 43 +++++++++++++++++++++++++++---- swarm/api/http/error_templates.go | 11 ++++++++ swarm/api/http/error_test.go | 40 ++++++++++++++++++++++++---- swarm/api/http/server.go | 22 ++++++++-------- 5 files changed, 98 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 0763d8492c..b8d2929016 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,9 @@ profile.cov # IdeaIDE .idea +# VS Code +.vscode + # dashboard /dashboard/assets/flow-typed /dashboard/assets/node_modules diff --git a/swarm/api/http/error.go b/swarm/api/http/error.go index 831cf23fe2..9a65412cf9 100644 --- a/swarm/api/http/error.go +++ b/swarm/api/http/error.go @@ -35,6 +35,7 @@ import ( //templateMap holds a mapping of an HTTP error code to a template var templateMap map[int]*template.Template +var caseErrors []CaseError //metrics variables var ( @@ -51,6 +52,13 @@ type ErrorParams struct { Details template.HTML } +//a custom error case struct that would be used to store validators and +//additional error info to display with client responses. +type CaseError struct { + Validator func(*Request) bool + Msg func(*Request) string +} + //we init the error handling right on boot time, so lookup and http response is fast func init() { initErrHandling() @@ -74,6 +82,29 @@ func initErrHandling() { //assign formatted HTML to the code templateMap[code] = template.Must(template.New(fmt.Sprintf("%d", code)).Parse(tname)) } + + caseErrors = []CaseError{ + { + Validator: func(r *Request) bool { return r.uri != nil && r.uri.Addr != "" && strings.HasPrefix(r.uri.Addr, "0x") }, + Msg: func(r *Request) string { + uriCopy := r.uri + uriCopy.Addr = strings.TrimPrefix(uriCopy.Addr, "0x") + return fmt.Sprintf(`The requested hash seems to be prefixed with '0x'. You will be redirected to the correct URL within 5 seconds.
+ Please click here if your browser does not redirect you.`, "/"+uriCopy.String()) + }, + }} +} + +//ValidateCaseErrors is a method that process the request object through certain validators +//that assert if certain conditions are met for further information to log as an error +func ValidateCaseErrors(r *Request) string { + for _, err := range caseErrors { + if err.Validator(r) { + return err.Msg(r) + } + } + + return "" } //ShowMultipeChoices is used when a user requests a resource in a manifest which results @@ -82,10 +113,10 @@ func initErrHandling() { //For example, if the user requests bzz://read and that manifest contains entries //"readme.md" and "readinglist.txt", a HTML page is returned with this two links. //This only applies if the manifest has no default entry -func ShowMultipleChoices(w http.ResponseWriter, r *http.Request, list api.ManifestList) { +func ShowMultipleChoices(w http.ResponseWriter, r *Request, list api.ManifestList) { msg := "" if list.Entries == nil { - ShowError(w, r, "Internal Server Error", http.StatusInternalServerError) + ShowError(w, r, "Could not resolve", http.StatusInternalServerError) return } //make links relative @@ -102,7 +133,7 @@ func ShowMultipleChoices(w http.ResponseWriter, r *http.Request, list api.Manife //create clickable link for each entry msg += "" + e.Path + "
" } - respond(w, r, &ErrorParams{ + respond(w, &r.Request, &ErrorParams{ Code: http.StatusMultipleChoices, Details: template.HTML(msg), Timestamp: time.Now().Format(time.RFC1123), @@ -115,13 +146,15 @@ func ShowMultipleChoices(w http.ResponseWriter, r *http.Request, list api.Manife //The function just takes a string message which will be displayed in the error page. //The code is used to evaluate which template will be displayed //(and return the correct HTTP status code) -func ShowError(w http.ResponseWriter, r *http.Request, msg string, code int) { +func ShowError(w http.ResponseWriter, r *Request, msg string, code int) { + additionalMessage := ValidateCaseErrors(r) if code == http.StatusInternalServerError { log.Error(msg) } - respond(w, r, &ErrorParams{ + respond(w, &r.Request, &ErrorParams{ Code: code, Msg: msg, + Details: template.HTML(additionalMessage), Timestamp: time.Now().Format(time.RFC1123), template: getTemplate(code), }) diff --git a/swarm/api/http/error_templates.go b/swarm/api/http/error_templates.go index 0457cb8a70..cc9b996ba4 100644 --- a/swarm/api/http/error_templates.go +++ b/swarm/api/http/error_templates.go @@ -168,6 +168,11 @@ func GetGenericErrorPage() string { {{.Msg}} + + + {{.Details}} + + @@ -342,6 +347,12 @@ func GetNotFoundErrorPage() string { {{.Msg}} + + + {{.Details}} + + + diff --git a/swarm/api/http/error_test.go b/swarm/api/http/error_test.go index c2c8b908b8..dc545722e1 100644 --- a/swarm/api/http/error_test.go +++ b/swarm/api/http/error_test.go @@ -18,12 +18,13 @@ package http_test import ( "encoding/json" - "golang.org/x/net/html" "io/ioutil" "net/http" "strings" "testing" + "golang.org/x/net/html" + "github.com/ethereum/go-ethereum/swarm/testutil" ) @@ -96,8 +97,37 @@ func Test500Page(t *testing.T) { defer resp.Body.Close() respbody, err = ioutil.ReadAll(resp.Body) - if resp.StatusCode != 500 || !strings.Contains(string(respbody), "500") { - t.Fatalf("Invalid Status Code received, expected 500, got %d", resp.StatusCode) + if resp.StatusCode != 404 { + t.Fatalf("Invalid Status Code received, expected 404, got %d", resp.StatusCode) + } + + _, err = html.Parse(strings.NewReader(string(respbody))) + if err != nil { + t.Fatalf("HTML validation failed for error page returned!") + } +} +func Test500PageWith0xHashPrefix(t *testing.T) { + srv := testutil.NewTestSwarmServer(t) + defer srv.Close() + + var resp *http.Response + var respbody []byte + + url := srv.URL + "/bzz:/0xthisShouldFailWith500CodeAndAHelpfulMessage" + resp, err := http.Get(url) + + if err != nil { + t.Fatalf("Request failed: %v", err) + } + defer resp.Body.Close() + respbody, err = ioutil.ReadAll(resp.Body) + + if resp.StatusCode != 404 { + t.Fatalf("Invalid Status Code received, expected 404, got %d", resp.StatusCode) + } + + if !strings.Contains(string(respbody), "The requested hash seems to be prefixed with") { + t.Fatalf("Did not receive the expected error message") } _, err = html.Parse(strings.NewReader(string(respbody))) @@ -127,8 +157,8 @@ func TestJsonResponse(t *testing.T) { defer resp.Body.Close() respbody, err = ioutil.ReadAll(resp.Body) - if resp.StatusCode != 500 { - t.Fatalf("Invalid Status Code received, expected 500, got %d", resp.StatusCode) + if resp.StatusCode != 404 { + t.Fatalf("Invalid Status Code received, expected 404, got %d", resp.StatusCode) } if !isJSON(string(respbody)) { diff --git a/swarm/api/http/server.go b/swarm/api/http/server.go index df90fd04f9..b8e7436cf0 100644 --- a/swarm/api/http/server.go +++ b/swarm/api/http/server.go @@ -336,7 +336,7 @@ func (s *Server) HandleGet(w http.ResponseWriter, r *Request) { key, err := s.api.Resolve(r.uri) if err != nil { getFail.Inc(1) - s.Error(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) + s.NotFound(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) return } @@ -421,7 +421,7 @@ func (s *Server) HandleGetFiles(w http.ResponseWriter, r *Request) { key, err := s.api.Resolve(r.uri) if err != nil { getFilesFail.Inc(1) - s.Error(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) + s.NotFound(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) return } @@ -494,7 +494,7 @@ func (s *Server) HandleGetList(w http.ResponseWriter, r *Request) { key, err := s.api.Resolve(r.uri) if err != nil { getListFail.Inc(1) - s.Error(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) + s.NotFound(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) return } @@ -598,7 +598,7 @@ func (s *Server) HandleGetFile(w http.ResponseWriter, r *Request) { key, err := s.api.Resolve(r.uri) if err != nil { getFileFail.Inc(1) - s.Error(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) + s.NotFound(w, r, fmt.Errorf("error resolving %s: %s", r.uri.Addr, err)) return } @@ -628,7 +628,7 @@ func (s *Server) HandleGetFile(w http.ResponseWriter, r *Request) { s.logDebug(fmt.Sprintf("Multiple choices! --> %v", list)) //show a nice page links to available entries - ShowMultipleChoices(w, &r.Request, list) + ShowMultipleChoices(w, r, list) return } @@ -693,7 +693,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // strictly a traditional PUT request which replaces content // at a URI, and POST is more ubiquitous) if uri.Raw() || uri.DeprecatedRaw() { - ShowError(w, r, fmt.Sprintf("No PUT to %s allowed.", uri), http.StatusBadRequest) + ShowError(w, req, fmt.Sprintf("No PUT to %s allowed.", uri), http.StatusBadRequest) return } else { s.HandlePostFiles(w, req) @@ -701,7 +701,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { case "DELETE": if uri.Raw() || uri.DeprecatedRaw() { - ShowError(w, r, fmt.Sprintf("No DELETE to %s allowed.", uri), http.StatusBadRequest) + ShowError(w, req, fmt.Sprintf("No DELETE to %s allowed.", uri), http.StatusBadRequest) return } s.HandleDelete(w, req) @@ -725,7 +725,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.HandleGetFile(w, req) default: - ShowError(w, r, fmt.Sprintf("Method "+r.Method+" is not supported.", uri), http.StatusMethodNotAllowed) + ShowError(w, req, fmt.Sprintf("Method "+r.Method+" is not supported.", uri), http.StatusMethodNotAllowed) } } @@ -757,13 +757,13 @@ func (s *Server) logError(format string, v ...interface{}) { } func (s *Server) BadRequest(w http.ResponseWriter, r *Request, reason string) { - ShowError(w, &r.Request, fmt.Sprintf("Bad request %s %s: %s", r.Method, r.uri, reason), http.StatusBadRequest) + ShowError(w, r, fmt.Sprintf("Bad request %s %s: %s", r.Request.Method, r.uri, reason), http.StatusBadRequest) } func (s *Server) Error(w http.ResponseWriter, r *Request, err error) { - ShowError(w, &r.Request, fmt.Sprintf("Error serving %s %s: %s", r.Method, r.uri, err), http.StatusInternalServerError) + ShowError(w, r, fmt.Sprintf("Error serving %s %s: %s", r.Request.Method, r.uri, err), http.StatusInternalServerError) } func (s *Server) NotFound(w http.ResponseWriter, r *Request, err error) { - ShowError(w, &r.Request, fmt.Sprintf("NOT FOUND error serving %s %s: %s", r.Method, r.uri, err), http.StatusNotFound) + ShowError(w, r, fmt.Sprintf("NOT FOUND error serving %s %s: %s", r.Request.Method, r.uri, err), http.StatusNotFound) }