server/etcdserver: fix oss-fuzz issue #13700
Conversation
| @@ -123,6 +124,9 @@ func (s *EtcdServer) applyV2Request(r *RequestV2, shouldApplyV3 membership.Shoul | |||
| alternative: func() string { return fmt.Sprintf("id:%d,method:%s,path:%s", r.ID, r.Method, r.Path) }, | |||
| } | |||
| defer func(start time.Time) { | |||
| if !utf8.ValidString(r.Method) { | |||
There was a problem hiding this comment.
Not sure about panic here.
Valid utf8 is just required to report metric. In normal code flow r.Method value that is not standard HTTP method (also not valid utf8) will result in ErrUnknownMethod being returned.
Problem happens when we want to report metrics about it. As it is already invalid request, maybe it's reasonable to just skip reporting metric?
Other approach would be to check if this whole code can be deleted. For the next release we are removing all V2 API. As this file is called apply_v2.go we should check if it's still called.
There was a problem hiding this comment.
@AdamKorcz also, isn't such a check impacts other places in the code? e.g. references to r.Method in different files, and in general.
There was a problem hiding this comment.
Thanks for the heads up @spzala, I will look into that this week.
Codecov Report
@@ Coverage Diff @@
## main #13700 +/- ##
==========================================
- Coverage 72.82% 72.61% -0.22%
==========================================
Files 465 465
Lines 37880 37882 +2
==========================================
- Hits 27588 27508 -80
- Misses 8517 8588 +71
- Partials 1775 1786 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Fixes https://oss-fuzz.com/testcase-detail/6750173361995776.
(Oss-fuzz issue ID has not yet been assigned)