Fix segfault if ce-specversion header missing

If a HTTP request does not contain the `ce-specversion` header then the
process of converting the request to an event returns a `nil`
event and a non-fatal error value. Unfortunately, in this situation,
the code continued to process the request resulting in an attempt to
de-reference the `nil` event value.

Fix by adding an additional condition to the error handling. Now
conversions that yield a non-fatal error must also return a non-nil
event, otherwise a HTTP 400 (Bad Request) status is returned.

Signed-off-by: Adrian Preston <prestona@uk.ibm.com>

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/Azure/go-autorest/autorest/to v0.2.0 // indirect
 	github.com/Azure/go-autorest/autorest/validation v0.1.0 // indirect
 	github.com/fortytw2/leaktest v1.3.0 // indirect
+	github.com/gogo/protobuf v1.2.0 // indirect
 	github.com/google/go-cmp v0.3.0
 	github.com/google/uuid v1.1.1
 	github.com/kelseyhightower/envconfig v1.4.0

pkg/cloudevents/transport/http/transport.go

@@ -546,13 +546,11 @@ func (t *Transport) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		Body:   body,
 	})
 	if err != nil {
-		isErr := true
+		isFatal := true
 		if txerr, ok := err.(*transport.ErrTransportMessageConversion); ok {
-			if !txerr.IsFatal() {
-				isErr = false
-			}
+			isFatal = txerr.IsFatal()
 		}
-		if isErr {
+		if isFatal || event == nil {
 			logger.Errorw("failed to convert http message to event", zap.Error(err))
 			w.WriteHeader(http.StatusBadRequest)
 			_, _ = w.Write([]byte(fmt.Sprintf(`{"error":%q}`, err.Error())))

pkg/cloudevents/transport/http/transport_test.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -220,3 +221,44 @@ func (h *namedHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	_, _ = w.Write([]byte(h.name))
 	h.next.ServeHTTP(w, r)
 }
+
+func TestServeHTTPSpecVersion(t *testing.T) {
+	tests := []struct {
+		specVersion    string
+		expectedStatus int
+	}{
+		{
+			// Missing specVersion -> HTTP 400
+			specVersion:    "",
+			expectedStatus: http.StatusBadRequest,
+		},
+		{
+			// Valid request
+			specVersion:    "0.3",
+			expectedStatus: http.StatusNoContent,
+		},
+		{
+			// Future specVersion -> HTTP 400
+			specVersion:    "999999.99",
+			expectedStatus: http.StatusBadRequest,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.specVersion, func(t *testing.T) {
+			transport := &cehttp.Transport{}
+			req := httptest.NewRequest("GET", "/", nil)
+			req.Header.Set("ce-specversion", test.specVersion)
+			w := httptest.NewRecorder()
+
+			transport.ServeHTTP(w, req)
+
+			actualStatus := w.Result().StatusCode
+			if actualStatus != test.expectedStatus {
+				t.Errorf("actual status (%d) != expected status (%d)", actualStatus, test.expectedStatus)
+				t.Errorf("response body: %s", w.Body.String())
+			}
+		})
+	}
+
+}