You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2011/07/30 04:33:45 UTC
DO NOT REPLY [Bug 51588] New: Change access modifiers in
AccessLogValve to make it easier to extend
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
Bug #: 51588
Summary: Change access modifiers in AccessLogValve to make it
easier to extend
Product: Tomcat 6
Version: 6.0.32
Platform: PC
OS/Version: Linux
Status: NEW
Severity: minor
Priority: P2
Component: Catalina
AssignedTo: dev@tomcat.apache.org
ReportedBy: rod@vagg.org
Classification: Unclassified
I'm adding some fields to a subclass of
org.apache.catalina.valves.AccessLogValve but createLogElements() is the
closest extension point available. Rather than reimplement the functionality
there it would be preferable if both of the createAccessLogElement() methods
were protected rather than private then it's a simple method of matching the
char you want to use and falling back to super.createAccessLogElement().
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 51588] Change access modifiers in AccessLogValve
to make it easier to extend
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
--- Comment #4 from Mark Thomas <ma...@apache.org> 2011-07-31 17:41:17 UTC ---
Methods changed in 7.0.x and will be included in 7.0.20 onwards.
Same change proposed for 6.0.x.
Even if Tomcat does support this particular use case, the general one of
extensibility is a valid one.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 51588] Change access modifiers in AccessLogValve
to make it easier to extend
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> 2011-07-30 12:18:47 UTC ---
Are those new fields specific for your environment? Maybe you can propose them
to be added to AccessLogValve?
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 51588] Change access modifiers in AccessLogValve
to make it easier to extend
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> 2011-07-30 13:41:54 UTC ---
Note, that
1. Parsing of X-Forwarded-For is already implemented in Tomcat 7 in
RemoteIpValve and RemoteIpFilter. RemoteIpValve was backported to Tomcat 6,
though maybe not all features of it.
2. AccessLogValve in Tomcat 7 can use request attributes that are set by
RemoteIpValve if you set AccessLogValve.requestAttributesEnabled property to
true.
3. You can set any attribute on the request object (even in the Valve itself)
and reference it later as %{xxx}r
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 51588] Change access modifiers in AccessLogValve
to make it easier to extend
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
--- Comment #2 from Rod <ro...@vagg.org> 2011-07-30 12:49:30 UTC ---
I want to add a %f and %F as possible options in the custom log pattern to log
IP address and name safely extracted from the X-Forwarded-For header with a
fallback to HttpServletRequest.getRemoteAddr().
Just to back up and clarify my original bug report in case it wasn't clear (now
that I re-read it I think I could have been clearer):
At the bottom of org.apache.catalina.valves.AccessLogValve, the last two
private methods are:
private AccessLogElement createAccessLogElement(char pattern)
and
private AccessLogElement createAccessLogElement(String header, char pattern)
If you want to add another 'field' to the custom log format pattern by
subclassing AccessLogValve then you have to go up to the 3rd last method:
protected AccessLogElement[] createLogElements()
but unfortunately it contains a lot of good logic that would have to be either
completely copied or duplicated in some way. Overriding one or both of the two
private methods lets you simply plug in your addition and not replace any of
the functionality of the original AccessLogValve.
This is the same for both the version 6 and 7 sources.
So my suggestions is simply to replace 'private' with 'protected' in the last 2
methods in AccessLogValve.
Of course I'd love the new options to be added to the Catalina core but I
suspected that they may be slightly peripheral? Perhaps not. I've just finished
writing up the details for anyone else that may have a use for this
functionality at:
http://rod.vagg.org/2011/07/handling-x-forwarded-for-in-java-and-tomcat/
If you think this would be useful to integrate into Catalina then I'd be more
than happy to refactor it all into AccessLogValve (it uses an additional
utility class as it is now) and post a patch here; please let me know what you
think about that option.
I think changing those access modifiers would be useful in either case by the
way.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 51588] Change access modifiers in AccessLogValve
to make it easier to extend
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51588
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #5 from Mark Thomas <ma...@apache.org> 2011-08-11 09:17:36 UTC ---
This has been fixed in 6.0.x and will be included in 6.0.33 onwards.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org