move most message handling code to java #85
Conversation
| } | ||
|
|
||
|
|
||
| private String readBytes(ByteBuf buf) { |
|
|
||
| private FullHttpResponse generateFailedResponse(HttpResponseStatus status) { | ||
| final FullHttpResponse response = new DefaultFullHttpResponse(req.protocolVersion(), status); | ||
| response.headers().set(HttpHeaderNames.CONTENT_LENGTH, 0); |
There was a problem hiding this comment.
the prior implementation sent "failed to authenticate" as the response body.
fwiw, i don't personally like the plain text response bodies but it could be non-passive to change it
There was a problem hiding this comment.
I don't think this message was very useful, so I thought of removing it, as it's unlikely someone will use the payload instead of the status code to determine success.
| final FullHttpResponse response = messageHandler.onNewMessage(remoteAddress, req); | ||
| final HttpResponse response; | ||
| final String token = req.headers().get(HttpHeaderNames.AUTHORIZATION); | ||
| if (messageHandler.validatesToken(token)) { |
There was a problem hiding this comment.
may want to short circuit this if
if (token != null && !token.isEmpty() && messageHandler.validatesToken(token))
There was a problem hiding this comment.
unfortunately we can't without moving more logic to java, because validatesToken has the reponsibility to both validate the token but only if it has been set in the configuration. short circuiting like this would mean that if a user doesn't set up creds in logstash any request without auth will return 401
| for(String key : stringHeaders.keySet()) { | ||
| headers.set(key, stringHeaders.get(key)); | ||
| } | ||
| response.headers().set(headers); |
There was a problem hiding this comment.
It looks like the authorization header is now being echo'ed back on the response. We should not echo it back since it is just encoded, not encrypted. (echo amFrZTpmb28= | base64 -D to see my super secret un/password)
"headers" => {
"authorization" => "Basic amFrZTpmb28=",
|
LGTM |
move the creation of most objects to java land
essentially the ruby side only invokes the codecs
performance bump of 15-20% on a simple noop pipeline (
bin/logstash -e "input { http {} } output {}")