netty: Netty server poorly handles unknown content type #3735
Conversation
ejona86
added
the
kokoro:run
kokoro-team
removed
the
kokoro:run
ejona86
left a comment
ejona86
left a comment
There was a problem hiding this comment.
Looks pretty good. Just one easy change. Thanks!
| // Remove the leading slash of the path and get the fully qualified method name | ||
| CharSequence path = headers.path(); | ||
| if (path.charAt(0) != '/') { | ||
| respondWithHttpError(ctx, streamId, 404, Status.Code.UNIMPLEMENTED, |
There was a problem hiding this comment.
The 404 case should be first. If both content-type and path are wrong, we should respond with 404.
|
moved the path check up to return 404 early. Added explicit |
ejona86
added
the
kokoro:run
kokoro-team
removed
the
kokoro:run
| return; | ||
| } | ||
|
|
||
| String method = path.subSequence(1, path.length()).toString(); |
There was a problem hiding this comment.
path is a CharSequence, so it doesn't have substring.
| String method = path.subSequence(1, path.length()).toString(); | ||
|
|
||
| // Verify that the Content-Type is correct in the request. | ||
| verifyContentType(streamId, headers); |
There was a problem hiding this comment.
Oops. I didn't delete these methods. They're now unused: determineMethod, verifyContentType
There was a problem hiding this comment.
I went ahead and deleted them, since it was trivial.
There was a problem hiding this comment.
@ejona86 great..thanks..I should have done it my self. Did not pay attention. Sorry :-)
ejona86
added
the
kokoro:run
kokoro-team
removed
the
kokoro:run
|
@ramaraochavali, thank you! |
ejona86
added
the
TODO:backport
ejona86
removed
the
TODO:backport
5 participants
PR for #3370