[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=45014

Mark Thomas  changed:

   What|Removed |Added

 CC||wolfg...@illmeyer.com

--- Comment #26 from Mark Thomas  ---
*** Bug 66533 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=45014

Mark Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WONTFIX

--- Comment #25 from Mark Thomas  ---
As per Remy's, closing as WONTFIX.

Note: There have been several changes to the Request API in 9.0.x in the 2+
years since comment #23.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

mr.oliver.hernan...@gmail.com changed:

   What|Removed |Added

  Attachment #32446|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #24 from mr.oliver.hernan...@gmail.com ---
Created attachment 32456
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=32456action=edit
Maven project for a valve that dumps the full request with the body

NOTE: For our use case, we needed a valve in JBoss, so this was coded against
the forked version of Tomcat within JBoss.  It should be simple to refactor for
use with the main trunk of Tomcat.

02/11/15 - fixed bug that was displaying post body incorrectly.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #18 from Remy Maucherat r...@apache.org ---
Adding your dummy classes means wrapping is supported and, probably, that there
is more API stability that what is actually the case. I continue to believe
that this is not a good idea. If people, like you, want to hack, feel free, but
it would be better to be able to disregard side effects and problems that could
be introduced. Especially if it is only hacking to be able to do useless
debugging.

Unless my veto is found invalid, I think this should be resolved as wontfix.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #19 from mr.oliver.hernan...@gmail.com ---
Remy, please re-read my previous comments
(https://issues.apache.org/bugzilla/show_bug.cgi?id=45014#c5 
https://issues.apache.org/bugzilla/show_bug.cgi?id=45014#c6).

This need is far from useless.  We're seeing these errors,

Caused by: java.io.IOException: Strange I/O stream, returned 0 bytes on read
at com.ctc.wstx.io.BaseReader.reportStrangeStream(BaseReader.java:216)
at com.ctc.wstx.io.UTF8Reader.loadMore(UTF8Reader.java:375)
at com.ctc.wstx.io.UTF8Reader.read(UTF8Reader.java:111)
at
com.ctc.wstx.io.ReaderBootstrapper.initialLoad(ReaderBootstrapper.java:250)
at
com.ctc.wstx.io.ReaderBootstrapper.bootstrapInput(ReaderBootstrapper.java:133)
at
com.ctc.wstx.stax.WstxInputFactory.doCreateSR(WstxInputFactory.java:546)
... 36 more

and the valve I put together has been a tremendous help in troubleshooting it.

Instead of this issue being categorized as a bug, maybe this should be
considered a feature request.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #21 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Remy Maucherat from comment #20)
 Yes, I did note that I guess in hindsight I could have implemented a
 filter. This is a feature for hackers, my vote is to let people deal with
 it to avoid having any additional support burden.

I'm happy to simply leave the classes as attachments to this enhancement
request and not commit anything. I don't personally have a use for them at this
moment. A few years ago it occurred to me that they would be something useful
for Valve authors. Since Valve is an internal API, Valve authors should already
be comfortable with an unstable API.

We'll see if this gets any votes. It's remained untouched for 6 years until
this week. It's clear there is not much demand that the examples here will not
fill.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #22 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Remy Maucherat from comment #18)
 Adding your dummy classes means wrapping is supported and, probably, that
 there is more API stability that what is actually the case.

This is the first time you've mentioned API stability which, honestly, is a
real concern.

Here are the stats for changes to o.a.coyote.Request:

- Tomcat 9, root to tip: no changes
- Tomcat 8, root (r1636341) to tip: no changes
- Tomcat 7, root (r1155369) to tip: 5 commits: r1446110, r1446138, r1446138,
*r156*, r1561675, r1518197. The single *starred* commit changed the API.

This API is pretty stable, even if it is not officially so.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #23 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Christopher Schultz from comment #22)
 - Tomcat 7, root (r1155369) to tip: 5 commits: r1446110, r1446138, r1446138,
 *r156*, r1561675, r1518197. The single *starred* commit changed the API.

This incorrectly identifies the commit which included the API change. The
correct commit is r1518197 for the API change.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #17 from mr.oliver.hernan...@gmail.com ---
I tend to agree more with Christopher, mainly because of this section of the
javadoc for the Valve interface that is misleading:

Consume bytes from the input stream associated with the Request, unless it is
completely generating the response, **OR WRAPPING THE REQUEST** before passing
it on.

When I read that, I had high hopes of implementing a custom valve easily.  Then
it was surprising that I could only find one example, Christopher's, of how to
wrap the request when a valve needs to consume data from the input stream, and
that Tomcat did not provide any wrapper class.  So I refactored an example
servlet filter from
http://architects.dzone.com/articles/debugging-soap-logging, and got it to work
as a valve.  The tricky part was figuring out that the HttpServletRequest is
doubly wrapped inside the org.apache.catalina.connector.Request class.

If the convenience of a built-in wrapper class will not be added, hopefully the
attached project will suffice for any others needing to do the same thing we
did.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #16 from mr.oliver.hernan...@gmail.com ---
Created attachment 32446
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=32446action=edit
Maven project for a valve that dumps the full request with the body

NOTE: For our use case, we needed a valve in JBoss, so this was coded against
the forked version of Tomcat within JBoss.  It should be simple to refactor for
use with the main trunk of Tomcat.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #20 from Remy Maucherat r...@apache.org ---
Yes, I did note that I guess in hindsight I could have implemented a filter.
This is a feature for hackers, my vote is to let people deal with it to avoid
having any additional support burden.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #9 from mr.oliver.hernan...@gmail.com ---
Thanks Christopher.  Your classes were a very good starting point for our use
case.  I have a custom valve implemented now that logs the request body like we
need.  I'll share it soon after I'm done tweaking it a bit, and of course
modifying it in a form suitable for sharing outside my closed source project.

Yes Remy, it took quite an effort to wrap the request in my valve, and after
looking at the internals of the API, I see your point about supporting this
being a big design change.  I guess in hindsight I could have implemented a
filter, but when I started this I liked the idea of a valve that intercepts the
request outside of the application.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

mr.oliver.hernan...@gmail.com changed:

   What|Removed |Added

 CC||mr.oliver.hernandez@gmail.c
   ||om

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #10 from Remy Maucherat r...@apache.org ---
Good thing it it works for you, but I'll have to veto its actual inclusion in
Tomcat: it would mean wrapping is possible and supported before the filter
chain, and it is not the case at the moment so user shouldn't be mislead.

Indeed, as you mention, wrapping in filters is supported and seems sufficient.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #8 from Remy Maucherat r...@apache.org ---
Wrapping is not possible until the filter chain is reached, it's really quite
simple. If you want to change that, it is a big design change that you could
advocate for the servlet container, but I would be against it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #11 from Christopher Schultz ch...@christopherschultz.net ---
Remy, please help me understand why one cannot wrap a Coyote request/response?

I don't see a reason why a Valve in the chain would not wrap the request or
response in a way similar to a Filter in the (later) chain.

Why would you be -1 for adding RequestWrapper/ResponseWrapper classes to
Tomcat? It seems quite natural to do so and I can't see anything in the
documentation that suggests it would break anything.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #12 from Remy Maucherat r...@apache.org ---
Please read again my previous comment for the veto justification.

I don't believe the classes can be wrapped without issues, usually wrapping
occurs to deal with the streams and that will likely not work well.

Indeed, the documentation doesn't say that wrapping is forbidden, but the
opposite is also true and is an obvious case where it would be mentioned if it
was allowed. I volunteer to add a note in the documentation to clarify.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #7 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to mr.oliver.hernandez from comment #5)
 We're actually using JBoss 7.4, which my understanding is that internally
 it's using a branched version of Tomcat 7.

You will have to wait probably a long time to get a patch from Red Hat. I'm
happy to just give you the files even if they don't make it into Tomcat.

 If it helps, we need a valve to troubleshoot why we're getting IOExceptions
 on incoming requests.  We're using proprietary, 3rd party routers in our
 environment, and we're pretty sure they're somehow mangling packets.  So we
 need to confirm this by outputting the full request, header and body, to
 then pass on to the provider of the routers for them to correct.

You could start with this:
http://tomcat.apache.org/tomcat-7.0-doc/config/filter.html#Request_Dumper_Filter

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #14 from Christopher Schultz ch...@christopherschultz.net ---
Remy, whether these classes are included or not, Valve authors can write their
own wrapper classes and wrap the Coyote request and response objects in them.
The only thing that is happening here is that Tomcat is providing the wrapper
classes instead of the Valve author having to write them manually.

Please note that these classes do nothing -- just like HttpServletWrapper. They
are expected to be a base class for extension so that the Valve author can
override whatever capabilities are appropriate for their use case. This could
be something simple like suppressing the transmission of a header value,
hooking-into the setStatus method, etc.

(I honestly don't understand your objections, having read them several times.
The main objection seems to be they haven't been available before... why
bother making them available now? i.e. placing the burden of proof on someone
else to settle the dispute. Your offer to change the documentation to support
your position is a bit underhanded.)

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #15 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Christopher Schultz from comment #14)
 Please note that these classes do nothing -- just like HttpServletWrapper.

That should have read HttpServletRequestWrapper/HttpServletResponseWrapper.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

Christopher Schultz ch...@christopherschultz.net changed:

   What|Removed |Added

  Attachment #21974|0   |1
is obsolete||

--- Comment #13 from Christopher Schultz ch...@christopherschultz.net ---
Created attachment 32444
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=32444action=edit
Updated patch for Tomcat trunk/9.0.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #6 from mr.oliver.hernan...@gmail.com ---
Oh, forgot to mention, our servers are SSL enabled, and the issue only occurs
when the requests are encrypted.  So, tools like tcpdump/tcpmon won't help, we
need to see the entire request after it's decrypted.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #5 from mr.oliver.hernan...@gmail.com ---
We're actually using JBoss 7.4, which my understanding is that internally it's
using a branched version of Tomcat 7.

Remy, can you please clarify?  What exactly is not the point?  Are you
referring to the org.apache.catalina.* core classes?  If so, what class(es)
will provide a convenient way to wrap a org.apache.catalina.connector.Request
instance that can then be passed on to getNext().invoke()?

If it helps, we need a valve to troubleshoot why we're getting IOExceptions on
incoming requests.  We're using proprietary, 3rd party routers in our
environment, and we're pretty sure they're somehow mangling packets.  So we
need to confirm this by outputting the full request, header and body, to then
pass on to the provider of the routers for them to correct.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #2 from mr.oliver.hernan...@gmail.com ---
I 100% agree that this should be added.

I'm running into the use case where I need to read a request's input stream,
and per the javadoc for Valve, I must wrap the request before passing it on.

It would be really convenient to either provide a wrapper in the API, or
re-design the API to the standard servlet interfaces and have invoke() accept a
reference to HttpServletRequest instead of the concrete class Request.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #3 from Christopher Schultz ch...@christopherschultz.net ---
Wow, a blast from the past. I had entirely forgotten about this.

I'll re-work the patch for trunk and see how far everything is interested in
back-porting. mr.oliver, what version of Tomcat are you using?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014

--- Comment #4 from Remy Maucherat r...@apache.org ---
That is clearly not the point. Up until the filter chain, the core classes
should be used.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

DO NOT REPLY [Bug 45014] Request and Response classes should have wrappers for use in Valves

[bugzilla]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45014





--- Comment #1 from Christopher Schultz [EMAIL PROTECTED]  2008-05-15 
15:02:59 PST ---
Created an attachment (id=21974)
 -- (https://issues.apache.org/bugzilla/attachment.cgi?id=21974)
Request and response wrapper class implementations

Attached RequestWrapper and ResponseWrapper class implementations.


-- 
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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]