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 2020/07/22 08:57:46 UTC

[Bug 64619] New: Regression: Removal of scratchdir fallback affects existing code

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

            Bug ID: 64619
           Summary: Regression: Removal of scratchdir fallback affects
                    existing code
           Product: Tomcat 8
           Version: 8.5.57
          Hardware: PC
            Status: NEW
          Severity: minor
          Priority: P2
         Component: Jasper
          Assignee: dev@tomcat.apache.org
          Reporter: schlm3@gmail.com
  Target Milestone: ----

https://github.com/apache/tomcat/commit/6d7e1a00cd706654eca445989d85960cbfba83ac
assumes, that the scratchdir is always set. But this is not necessarily the
case, especially for tomcat embedded.

Adding this code change in a new mayor release along with an entry in the
changelog/releasenotes wouldn't be a problem.
But adding this change in a minor release, without notice in the changelog and
without a real need (it does not fix a bug or security issue) IS a problem. 

Users of Tomcat-embedded should be able to rely on the changelog, such that
applied security fixes can be integrated as fast as possible into the dependent
applications.
Therefore, please don't do unforced refactorings in a minor maintenance
release.

ServletContext is an interface which can have many sorts of implementations. We
use our own implementation of that interface for various reasons. You can not
rely on the assumption, that the Attribute ServletContext.TEMPDIR is always
set.

-- 
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 64619] Regression: Removal of scratchdir fallback affects existing code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64619

--- Comment #3 from Markus Schlegel <sc...@gmail.com> ---
Thanks for the clarification.
I can understand your demand for having the codebase of the maintenance
branches somewhat in sync.
Since the checkin was not associated with a issue, I thought that it was an
unforced change.

After looking into the spec given by markt, I understand that it indeed was a
bug to silently share the same working-directory across different servlet
context when javax.servlet.context.tempdir is not defined.
Having a "must-have" attribute defined in the servlet spec seems very unfortune
for me. The spec would better have made an additional method on the interface
for this, such that the change and need becomes obvious.

But finally, I will have to fix that in our code.
Nevertheless, it would have been a good practice to create an issue for this
code change and explain the reasoning for the change there (with reference to
specification). My application would still have failed to run, but it would
have been clear why.

@mgrigorov: We have different usages of Tomcat in our Application. The "normal"
WebServer/Servlet-Container usage is not affected by this issue. Another usage
uses only Jasper as a component of Tomcat to generate reports using JSP's
without being inside a real web-container. Only minor involvement of catalina
there and therefore no call to postWorkDirectory().

-- 
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 64619] Regression: Removal of scratchdir fallback affects existing code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64619

mgrigorov <mg...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from mgrigorov <mg...@apache.org> ---
This change has been made first in Tomcat 10.x (master) branch and then
downported to 8.5.x to keep the codebases in sync.


Tomcat sets the attribute to the ServletContext at
org.apache.catalina.core.StandardContext#postWorkDirectory(). 

StandardContext is a facade for the implementation of ServletContext. So it
does not matter whether you use a custom impl of ServletContext or not. The
question is: Why org.apache.catalina.core.StandardContext#postWorkDirectory()
is not called in your case ?
postWorkDirectory() is called by
org.apache.catalina.core.StandardContext#startInternal()

-- 
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 64619] Regression: Removal of scratchdir fallback affects existing code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64619

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |WONTFIX
             Status|NEW                         |RESOLVED

-- 
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 64619] Regression: Removal of scratchdir fallback affects existing code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64619

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
The Tomcat code base being the age and size it is, it isn't uncommon to come
across cruft while researching other issues. The decision when and how to
remove such cruft is always going to be taken on a case by case basis.

In this case it is a mandatory requirement of the Servlet specification (see
Servlet 3.1, section 4.8.1) that a ServletContext implementation provides a
temporary storage location and exposes it via the javax.servlet.context.tempdir
attribute. While the Servlet 3.1 spec applies to Tomcat 8.x, the same
requirement has existed at least as far back as Servlet 2.2 (December 1999 /
Tomcat 3).

Given the above, removal of the code intended to address a situation that
should not have existed for over 20 years and looked to be addressing pre
Tomact 3 implementations seemed reasonable.

Whether or not the clean-up should have been included in the changelog is
debatable. With hindsight, inclusion may have helped in this case. There is a
balance to strike between including everything in the changelog and not
overwhelming the changelog with trivial entries. I think there is probably a
case for moving that balance a little more towards including entries.

At this point this looks more like a specification compliance bug in the custom
ServletContext implementation rather than a Tomcat issue.

-- 
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