Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrp validation update #318

Merged
merged 13 commits into from
Nov 14, 2023
29 changes: 17 additions & 12 deletions primaryHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@
otelmux.WithPropagators(tracing.Propagator()),
otelmux.WithTracerProvider(tracing.TracerProvider()),
}
router.Use(otelmux.Middleware("mainSpan", otelMuxOptions...), candlelight.EchoFirstTraceNodeInfo(tracing.Propagator(), true), ValidateWRP())
router.Use(otelmux.Middleware("mainSpan", otelMuxOptions...), candlelight.EchoFirstTraceNodeInfo(tracing.Propagator(), true), ValidateWRP(logger))

Check warning on line 395 in primaryHandler.go

View check run for this annotation

Codecov / codecov/patch

primaryHandler.go#L395

Added line #L395 was not covered by tests

router.NotFoundHandler = http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) {
xhttp.WriteError(response, http.StatusBadRequest, "Invalid endpoint")
Expand Down Expand Up @@ -563,26 +563,31 @@
})
}

func ValidateWRP() func(http.Handler) http.Handler {
func ValidateWRP(logger *zap.Logger) func(http.Handler) http.Handler {

Check warning on line 566 in primaryHandler.go

View check run for this annotation

Codecov / codecov/patch

primaryHandler.go#L566

Added line #L566 was not covered by tests
denopink marked this conversation as resolved.
Show resolved Hide resolved
return func(delegate http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

if msg, ok := wrpcontext.GetMessage(r.Context()); ok {
validators := wrp.SpecValidators()
var err error
denopink marked this conversation as resolved.
Show resolved Hide resolved

validators := wrp.SpecValidators()

Check warning on line 573 in primaryHandler.go

View check run for this annotation

Codecov / codecov/patch

primaryHandler.go#L572-L573

Added lines #L572 - L573 were not covered by tests
for _, v := range validators {
err = multierr.Append(err, v.Validate(*msg))
}

if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this nil check since errors.Is should handle this

Suggested change
if err != nil {
if err != nil {

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(
w,
`{"code": %d, "message": "%s"}`,
http.StatusBadRequest,
fmt.Sprintf("failed to validate WRP message: %s", err),
)
return
if errors.Is(err, wrp.ErrorInvalidMessageEncoding.Err) || errors.Is(err, wrp.ErrorInvalidMessageType.Err) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(
w,
`{"code": %d, "message": "%s"}`,
http.StatusBadRequest,
fmt.Sprintf("failed to validate WRP message: %s", err))
return
} else if errors.Is(err, wrp.ErrorInvalidDestination) || errors.Is(err, wrp.ErrorInvalidSource.Err) {
logger.Warn("WRP message validation failures found", zap.Error(err))
}

Check warning on line 590 in primaryHandler.go

View check run for this annotation

Codecov / codecov/patch

primaryHandler.go#L579-L590

Added lines #L579 - L590 were not covered by tests
}
}

Expand Down