From f15bc3fd7b013f2f31e216cd55a587d79c453c32 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Tue, 1 Aug 2017 13:51:26 +1000 Subject: [PATCH 1/2] Add test coverage for disallowed methods on root --- csp_collector_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 csp_collector_test.go diff --git a/csp_collector_test.go b/csp_collector_test.go new file mode 100644 index 0000000..caa3fc0 --- /dev/null +++ b/csp_collector_test.go @@ -0,0 +1,29 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestHandlerForDisallowedMethodsOnRoot(t *testing.T) { + disallowedMethods := []string{"GET", "DELETE", "PUT", "TRACE", "PATCH"} + + for _, method := range disallowedMethods { + t.Run(method, func(t *testing.T) { + request, err := http.NewRequest(method, "localhost:8080/", nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + recorder := httptest.NewRecorder() + handleViolationReport(recorder, request) + + response := recorder.Result() + defer response.Body.Close() + + if response.StatusCode != http.StatusMethodNotAllowed { + t.Errorf("Expected HTTP status %v; got %v", http.StatusMethodNotAllowed, response.Status) + } + }) + } +} From ffd1c19ba98c851de05d0c58dedf5c7892668867 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 4 Oct 2017 10:03:37 +1100 Subject: [PATCH 2/2] Don't `panic` for invalid payloads Initially I was using the `panic` as a means of debugging however now this has an adequate test suite and a far bit of production usage, this can now just render a HTTP 422 instead. --- csp_collector.go | 5 +++-- csp_collector_test.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/csp_collector.go b/csp_collector.go index 5d37447..e98f8b3 100644 --- a/csp_collector.go +++ b/csp_collector.go @@ -64,7 +64,8 @@ func handleViolationReport(w http.ResponseWriter, r *http.Request) { err := decoder.Decode(&report) if err != nil { - panic(err) + w.WriteHeader(http.StatusUnprocessableEntity) + return } defer r.Body.Close() @@ -107,7 +108,7 @@ func validateViolation(r CSPReport) error { for _, value := range ignoredBlockedURIs { if strings.HasPrefix(r.Body.BlockedURI, value) == true { - err := fmt.Errorf("Blocked URI ('%s') is an invalid resource.", value) + err := fmt.Errorf("blocked URI ('%s') is an invalid resource", value) return err } } diff --git a/csp_collector_test.go b/csp_collector_test.go index 439ea6d..fd15af8 100644 --- a/csp_collector_test.go +++ b/csp_collector_test.go @@ -137,7 +137,7 @@ func TestValidateViolationWithInvalidBlockedURIs(t *testing.T) { t.Errorf("expected error to be raised but it didn't") } - if validateErr.Error() != fmt.Sprintf("Blocked URI ('%s') is an invalid resource.", blockedURI) { + if validateErr.Error() != fmt.Sprintf("blocked URI ('%s') is an invalid resource", blockedURI) { t.Errorf("expected error to include correct message string but it didn't") } })