Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sada-sigsci committed Nov 15, 2024
1 parent 932a76f commit d4fb21b
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 19 deletions.
7 changes: 6 additions & 1 deletion module.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@ import (
"sync"
"time"

"github.com/signalsciences/sigsci-module-golang/schema"
"github.com/signalsciences/tlstext"
)

type RPCMsgIn = schema.RPCMsgIn
type RPCMsgIn2 = schema.RPCMsgIn2
type RPCMsgOut = schema.RPCMsgOut

// Module is an http.Handler that wraps an existing handler with
// data collection and sends it to the Signal Sciences Agent for
// inspection.
Expand Down Expand Up @@ -115,7 +120,7 @@ func (m *Module) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

rw := NewResponseWriter(w, out.RespActions)
rw := newResponseWriter(w, out.RespActions)

wafresponse := out.WAFResponse
switch {
Expand Down
18 changes: 12 additions & 6 deletions responsewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"io"
"net"
"net/http"

"github.com/signalsciences/sigsci-module-golang/schema"
)

// ResponseWriter is a http.ResponseWriter allowing extraction of data needed for inspection
Expand All @@ -23,7 +25,11 @@ type ResponseWriterFlusher interface {
}

// NewResponseWriter returns a ResponseWriter or ResponseWriterFlusher depending on the base http.ResponseWriter.
func NewResponseWriter(base http.ResponseWriter, actions []Action) ResponseWriter {
func NewResponseWriter(base http.ResponseWriter) ResponseWriter {
return newResponseWriter(base, nil)
}

func newResponseWriter(base http.ResponseWriter, actions []schema.Action) ResponseWriter {
// NOTE: according to net/http docs, if WriteHeader is not called explicitly,
// the first call to Write will trigger an implicit WriteHeader(http.StatusOK).
// this is why the default code is 200 and it only changes if WriteHeader is called.
Expand All @@ -43,7 +49,7 @@ type responseRecorder struct {
base http.ResponseWriter
code int
size int64
actions []Action
actions []schema.Action
}

// BaseResponseWriter returns the base http.ResponseWriter allowing access if needed
Expand Down Expand Up @@ -79,15 +85,15 @@ func (w *responseRecorder) mergeHeader() {
hdr := w.base.Header()
for _, a := range w.actions {
switch a.Code {
case AddHdr:
case schema.AddHdr:
hdr.Add(a.Args[0], a.Args[1])
case SetHdr:
case schema.SetHdr:
hdr.Set(a.Args[0], a.Args[1])
case SetNEHdr:
case schema.SetNEHdr:
if len(hdr.Get(a.Args[0])) == 0 {
hdr.Set(a.Args[0], a.Args[1])
}
case DelHdr:
case schema.DelHdr:
hdr.Del(a.Args[0])
}
}
Expand Down
18 changes: 10 additions & 8 deletions responsewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"net/http/httptest"
"reflect"
"testing"

"github.com/signalsciences/sigsci-module-golang/schema"
)

// testResponseRecorder is a httptest.ResponseRecorder without the Flusher interface
Expand Down Expand Up @@ -117,12 +119,12 @@ func testResponseWriter(t *testing.T, w ResponseWriter, flusher bool) {

// TestResponseWriter tests a non-flusher ResponseWriter
func TestResponseWriter(t *testing.T) {
testResponseWriter(t, NewResponseWriter(&testResponseRecorder{httptest.NewRecorder()}, nil), false)
testResponseWriter(t, NewResponseWriter(&testResponseRecorder{httptest.NewRecorder()}), false)
}

// TestResponseWriterFlusher tests a flusher ResponseWriter
func TestResponseWriterFlusher(t *testing.T) {
testResponseWriter(t, NewResponseWriter(&testResponseRecorderFlusher{httptest.NewRecorder()}, nil), true)
testResponseWriter(t, NewResponseWriter(&testResponseRecorderFlusher{httptest.NewRecorder()}), true)
}

func TestResponseHeaders(t *testing.T) {
Expand All @@ -134,13 +136,13 @@ func TestResponseHeaders(t *testing.T) {
"X-Report": []string{"bb"},
},
}
actions := []Action{
{AddHdr, []string{"csp", "src=abc"}},
{SetHdr, []string{"content-type", "text/json"}},
{DelHdr, []string{"x-powered-by"}},
{SetNEHdr, []string{"x-report", "cc"}},
actions := []schema.Action{
{schema.AddHdr, []string{"csp", "src=abc"}},
{schema.SetHdr, []string{"content-type", "text/json"}},
{schema.DelHdr, []string{"x-powered-by"}},
{schema.SetNEHdr, []string{"x-report", "cc"}},
}
NewResponseWriter(resp, actions).Write([]byte("foo"))
newResponseWriter(resp, actions).Write([]byte("foo"))

got := resp.Header()
expected := http.Header{
Expand Down
2 changes: 1 addition & 1 deletion rpc.go → schema/rpc.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package sigsci
package schema

//go:generate go run github.com/tinylib/[email protected] -unexported -tests=false

Expand Down
2 changes: 1 addition & 1 deletion rpc_gen.go → schema/rpc_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ go test .

rm -rf artifacts/
mkdir -p artifacts/sigsci-module-golang
cp -rf \
cp --parents -rf \
VERSION CHANGELOG.md LICENSE.md README.md \
clientcodec.go rpc.go rpc_gen.go rpcinspector.go inspector.go responsewriter.go module.go version.go config.go \
clientcodec.go schema/rpc.go schema/rpc_gen.go rpcinspector.go inspector.go responsewriter.go module.go version.go config.go \
responsewriter_test.go module_test.go config_test.go \
examples \
artifacts/sigsci-module-golang/
Expand Down

0 comments on commit d4fb21b

Please sign in to comment.