search

rabbitmq / rabbitmq-server

Open source RabbitMQ: core server and tier 1 (built-in) plugins
https://www.rabbitmq.com/
Other
12.3k stars 3.91k forks source link

Remove "\n" from the "Access refused for user 'xxxxxx'\n" log string #2568

Closed johnfoldager closed 1 year ago

johnfoldager commented 8 years ago

In the logs we see this:

=ERROR REPORT==== 5-Apr-2016::09:33:10 ===
STOMP error frame sent:
Message: "Bad CONNECT"
Detail: "Access refused for user 'USERNAME'\n"
Server private detail: none

It would be nice to have the "\n" removed from the output because it confuses and others might also think that the username might contain both quotes and \n in the client application.

As far as I can see it is related to line 579 of https://github.com/rabbitmq/rabbitmq-stomp/blob/6fa61a6880449ff5c0c86ba017f5f9399f3592c4/src/rabbit_stomp_processor.erl

michaelklishin commented 8 years ago

This line feed likely exists for a reason: STOMP frames are allowed to end in a line feed character by the spec, even though one client library breaks when they do.

It doesn't mean we need to add it to log entries, of course.

johnfoldager commented 8 years ago

We know about the linefeed in the STOMP frames, however this is a log entry and because of this extra \n no one actually knows if: 'USERNAME'\n -- \n is a bug in the writing of the log entry, or 'USERNAME'\n -- the client tried to log in using 'USERNAME'\n or just USERNAME

Also because the entire string is encapsulated in double-quotes (") then - even though the \n is removed - we still can't see if the user is 'USERNAME' or USERNAME (with or without quotes). Maybe the log line should simply be change to:

Detail: "Access refused for user: USERNAME"

Notice the colon (:), no single-quote and no \n. I wouldn't assume that the end-double-quote (") being part of the username as we already have the beginning-double-quote after Detail: