[UNDERTOW-2370] Guard against non-servlet ctx request with ServletNam…#1758
[UNDERTOW-2370] Guard against non-servlet ctx request with ServletNam…#1758baranowb wants to merge 1 commit intoundertow-io:mainfrom
Conversation
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.
There was a problem hiding this comment.
@baranowb if other attributes return null and it is printed as '-', I believe we need to return null here as well
| if (src != null) { | ||
| return src.getCurrentServlet().getManagedServlet().getServletInfo().getName(); | ||
| } else { | ||
| return UNDEFINED; |
There was a problem hiding this comment.
@baranowb if other attributes return null and it is printed as '-', I believe we need to return null here as well
fl4via
added
the
waiting PR update
…eAttribute in access log
baranowb
removed
the
waiting PR update
3 participants
…eAttribute in access log
Issue: https://issues.redhat.com/browse/UNDERTOW-2370