[UNDERTOW-2370] Guard against non-servlet ctx request with ServletNam… #1758
Conversation
…eAttribute in access log
baranowb
added
bug fix
| if (src != null) { | ||
| return src.getCurrentServlet().getManagedServlet().getServletInfo().getName(); | ||
| } else { | ||
| return UNDEFINED; |
There was a problem hiding this comment.
Could you explain the reasoning behind intentionally returning "No-Servlet" here?
I was thinking that returning null would be sufficient, as it gets logged as - in the access log by default. This would also align with other ExchangeAttributes, which typically return null when a value isn't available.
There was a problem hiding this comment.
UX, looks better than "null".
There was a problem hiding this comment.
To clarify, my suggestion of returning null does not result in the string "null" being logged. Instead, the access log mechanism converts it to -, which is the standard convention for an empty value in Undertow.
My main concern is consistency. All other ExchangeAttributes (like RequestHeaderAttribute, ResponseHeaderAttribute, RemoteUserAttribute, etc.) return null and are logged as -. Introducing a custom "No-Servlet" string here would create a special case and make the logs less uniform.
For this reason, I still believe that returning null is the most appropriate approach.
2 participants
…eAttribute in access log
Issue: https://issues.redhat.com/browse/UNDERTOW-2370