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 2018/09/01 13:51:49 UTC
[Bug 62669] New: ResponseIncludeWrapper.getContentType() never
returns NULL and sets the field
https://bz.apache.org/bugzilla/show_bug.cgi?id=62669
Bug ID: 62669
Summary: ResponseIncludeWrapper.getContentType() never returns
NULL and sets the field
Product: Tomcat 8
Version: 8.5.x-trunk
Hardware: All
OS: All
Status: NEW
Severity: normal
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: svenuhlig@web.de
Target Milestone: ----
Hello,
according to the documentation
org.apache.catalina.ssi.ResponseIncludeWrapper.getContentType() should return
NULL if the content type is not known. However a fallback to
"application/x-octet-stream" is implemented and it actually sets the content
type field of the object.
I had a quick look into the SVN repository and it seems this code was
introduced in or before 2006. Because this was 12 years ago and because I could
not find any report regarding this issue, I guess everyone else can work with
that and only the documentation needs to be adapted.
However I actually prefer to get NULL. First of all for me it is bad practice
to set a value in a getter (if it is not called "lazy..."). But let me try to
explain the real problem.
The control flow of SSIFilter is:
* ...
* Wrap the actual response with ResponseIncludeWrapper
* Continue and complete the filter chain
* Check the content type
* ...
The problem is when the filter chain is continued and completed, other filters
that get the content type from the response (ResponseIncludeWrapper) actually
change the content type of the response even though this is most likely NOT
what the other filters want to do. It happens by accident, just by calling
getContentType.
Also some filters rely on the fact that the getContentType returns NULL if it
is not known, in order to check if they should set it on their own. They never
set the content type because with the current implementation it always return a
value other than NULL.
Best regards,
Sven.
--
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
[Bug 62669] ResponseIncludeWrapper.getContentType() never returns
NULL and sets the field
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62669
--- Comment #2 from Mark Thomas <ma...@apache.org> ---
I'm currently struggling to see why ResposneIncludeWrapper goes to the trouble
it does to capture the content type and the last modified date.
I worry I am missing something so I want to look at this some more but I am
currently leaning towards removing all of that code and simply obtaining the
content type and last modified date from the response object. That will mean
handling a potential NPE but that is one line of code vs rather more in the
current implementation.
--
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
[Bug 62669] ResponseIncludeWrapper.getContentType() never returns
NULL and sets the field
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62669
--- Comment #3 from Sven <sv...@web.de> ---
I am not a maintainer or contributor to this project yet but I also suggestion
to remove the wrapper.
Currently I simply add another filter after SSIFilter that wraps the
ResponseIncludeWrapper again and return the original response's content type as
a work-around for this issue. So far it works well but to be honest I did no
in-depth testing.
--
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
[Bug 62669] ResponseIncludeWrapper.getContentType() never returns
NULL and sets the field
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62669
--- Comment #1 from Mark Thomas <ma...@apache.org> ---
Going back this far in svn requires a little knowledge of the project history
and how the repos have been structured over time.
There was a big re-organisation between 5.5.x and 6.0.x. The files were copied
without history so if you want to go back further than 6.0.x you need to look
into 5.5.x (then 5.0.x and then 4.1.x).
Looking in 5.5.x shows that this change was part of a patch submitted under bug
33106. Next step is to check that bug, review the patch and see if there is a
reason for this change.
--
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
[Bug 62669] ResponseIncludeWrapper.getContentType() never returns
NULL and sets the field
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62669
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- trunk for 9.0.12 onwards
- 8.5.x for 8.5.34 onwards
- 7.0.x for 7.0.91 onwards
I've removed the Content-Type processing from the wrapper.
I left the last modified processing as it did serve a purpose (it saved looking
for and parsing the header to pass the value to the SSIProcessor).
--
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