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/03/30 10:56:18 UTC
[Bug 64290] New: Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
Bug ID: 64290
Summary: Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Product: Tomcat 8
Version: 8.5.x-trunk
Hardware: PC
Status: NEW
Severity: normal
Priority: P2
Component: Util
Assignee: dev@tomcat.apache.org
Reporter: schlm3@gmail.com
Target Milestone: ----
The commit 218ea934fc71a0948c1b2e313e9cf20dede2cc23 from December 6, 2019 moved
some Innerclasses from org.apache.tomcat.util.http.fileupload.FileUploadBase to
the package org.apache.tomcat.util.http.fileupload.impl .
Since some of those classes were public in FileUploadBase, they might have been
used somewhere in other projects. Our Application for example uses
org.apache.tomcat.util.http.fileupload.FileUploadBase.FileSizeLimitExceededException
.
This now ends in the fact, that compiled code will not run with newer 8.5.x
Versions of Tomcat. As long as such projects or libraries are not fixed,
customers cannot upgrade to newest Tomcat versions which will eventually end up
in tomcat installations with known security issues.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #2 from Remy Maucherat <re...@apache.org> ---
Fileupload as it is included is considered the same as if it was internal
Tomcat code, so releases or snapshots are not relevant.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #3 from Markus Schlegel <sc...@gmail.com> ---
"Do you
org.apache.tomcat.util.http.fileupload.FileUploadBase.FileSizeLimitExceededException
from #getCause()?"
We are using Tomcat Embedded and we have built a Jax-Rs resource for the
Fileupload, which relies on the org.apache.tomcat.util.http.fileupload.*
classes.
From org.apache.tomcat.util.http.fileupload.package-info.java I can read:
"While this package provides the generic functionality for file uploads, these
classes are not typically used directly. Instead, normal usage involves one of
the provided extensions of ...."
This 1.) does not state, that this is not part of the official API and
2.) is not obvious for a developer using one of the classes/innerclasses inside
the package.
The refactored new version of it makes it better, because it places inofficial
classes inside an "impl" package, which makes that facto obvious.
After all, the changed code is what it should look like, but you should not do
this with incompatible changes in a minor release and even worse, not without
notice in the changelog.
If you think this kind of problem is isolated to our special usecase, then no
further action may be required, since we can change our maintenance code to not
rely on that classes anymore.
But if other might use these classes, they will start to get errors as soon as
they updated to a newer Tomcat version and use that functionality.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #9 from Markus Schlegel <sc...@gmail.com> ---
I will change our code. Thank you guys for digging into this!
I'll also check if we used some other inofficial classes. When I got you right,
we should not use the tomcat classes at all but rather the apache-commons ones,
correct? That's somewhat sad because behavior between tomcat built-in code and
our addons might differ because of the different versions used.
Also, I'm somewhat confused about what parts of tomcat-api are official or not.
From the release-notes I take, that only javax.** and org.apache.catalina.*
(without subpackages) are considered stable. From the comments of you guys I
take, that you assume that only these classes make up the official API (which
is not necessary the case).
But:
1. If "release-notes" is right, what about using classes from org.apache.juli
as stated in https://tomcat.apache.org/tomcat-8.5-doc/logging.html ? Do people
following your documentation have to use inoffical API's ?
2. Why do you provide an official javadocs page which provides Javadocs for
internal, non-official packages which should not be used outside of tomcat ?
Wouldn't it be clearer if you would provide API-Doc only for official API's?
3. Are javadoc-comments like "This package contains various IO related utility
classes or methods, which are basically reusable and not necessarily restricted
to the scope of a file upload." (from
org.apache.tomcat.util.http.fileupload.util) wrong?
Thanks for clearing this.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #4 from Michael Osipov <mi...@apache.org> ---
(In reply to Remy Maucherat from comment #2)
> Fileupload as it is included is considered the same as if it was internal
> Tomcat code, so releases or snapshots are not relevant.
If so, we should not add them as root causes in public classes:
> $ grep -r -E -e 'org.apache.tomcat.util.http.fileupload.impl.+Exception' java
> java/org/apache/catalina/connector/Request.java:import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
> java/org/apache/catalina/connector/Request.java:import org.apache.tomcat.util.http.fileupload.impl.SizeException;
> } catch (InvalidContentTypeException e) {
> parameters.setParseFailedReason(FailReason.INVALID_CONTENT_TYPE);
> partsParseException = new ServletException(e);
> } catch (SizeException e) {
> parameters.setParseFailedReason(FailReason.POST_TOO_LARGE);
> checkSwallowInput();
> partsParseException = new IllegalStateException(e);
> } catch (FileUploadException e) {
> parameters.setParseFailedReason(FailReason.IO_ERROR);
> partsParseException = new IOException(e);
>
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #5 from Mark Thomas <ma...@apache.org> ---
Tomcat is going to continue to track HEAD for the various Apache Commons
projects for which a local fork is maintained.
Generally, a class being public doesn't make it part of Tomcat's public API.
Tomcat's public API is defined in the RELEASE-NOTES file at the root of both
the source and binary distributions. That said, we try to provide a lot more
stability than the guarantees in that file.
I can see why applications might be interested in the specific exception
thrown. There are (ugly) ways to write code to handle both variations of class
structure but in this instance I think it would be reasonable if we added back
inner classes (deprecated and marked for removal in Tomcat 10) on an "as
required" basis and have then simply extend the new classes.
Markus, I'm assuming that the only class you need is
FileUploadBase.FileSizeLimitExceededException
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #10 from Mark Thomas <ma...@apache.org> ---
There isn't a clear dividing line between what is and is not part of the public
API.
The only absolute guarantees are what is in the release notes. Those are
deliberately minimal. Everything else we try not to change but there are times
when we do make changes.
We might make changes to add new features, fix a security issue, update a
dependency sometimes. We try to keep changes backwards compatible but sometimes
we decide the complexity to maintain backwards compatibility isn't justified.
Generally, the deeper you go into the Tomcat internals, the more likely you
will, at some point, get tripped up by one of these changes.
There is a lot of judgement involved and it is inevitable we won't always get
it right. The safety net is that we do monthly releases so if an issue is
identified we can correct it fairly swiftly. Related to that, each release
normally has a voting Window of at least 72 hours. If users test those release
candidates and find issues we can fix them and restart the release process if
necessary.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #6 from Michael Osipov <mi...@apache.org> ---
(In reply to Markus Schlegel from comment #3)
> "Do you
> org.apache.tomcat.util.http.fileupload.FileUploadBase.
> FileSizeLimitExceededException from #getCause()?"
>
> We are using Tomcat Embedded and we have built a Jax-Rs resource for the
> Fileupload, which relies on the org.apache.tomcat.util.http.fileupload.*
> classes.
Very bad, you should solely rely on Servet API
> From org.apache.tomcat.util.http.fileupload.package-info.java I can read:
> "While this package provides the generic functionality for file uploads,
> these classes are not typically used directly. Instead, normal usage
> involves one of the provided extensions of ...."
>
> This 1.) does not state, that this is not part of the official API and
> 2.) is not obvious for a developer using one of the classes/innerclasses
> inside the package.
Granted.
> The refactored new version of it makes it better, because it places
> inofficial classes inside an "impl" package, which makes that facto obvious.
>
> After all, the changed code is what it should look like, but you should not
> do this with incompatible changes in a minor release and even worse, not
> without notice in the changelog.
Fully agree.
> If you think this kind of problem is isolated to our special usecase, then
> no further action may be required, since we can change our maintenance code
> to not rely on that classes anymore.
Please do so regardless of this issue.
At the end, if you want to be stable or use explicit exceptions, use Commons
File Upload directly.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |WONTFIX
--- Comment #8 from Mark Thomas <ma...@apache.org> ---
There is a changelog entry:
<quote>
Update the internal fork of Apache Commons FileUpload to 2317552 (2019-12-06,
2.0-SNAPSHOT). Refactoring. (markt)
</quote>
It doesn't make the API change explicit but the possibility is implied.
I'd prefer not to make the change at all - hence my preference/offer to look at
this on an "as required" basis rather than just applying a general fix.
Given that you are going to change / have changed your code to account for this
I'm going to resolve this as WONTFIX but if we see more reports along these
lines we can look again at adding back those inner classes as required.
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
Michael Osipov <mi...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
OS| |All
--- Comment #1 from Michael Osipov <mi...@apache.org> ---
(In reply to Markus Schlegel from comment #0)
> The commit 218ea934fc71a0948c1b2e313e9cf20dede2cc23 from December 6, 2019
> moved some Innerclasses from
> org.apache.tomcat.util.http.fileupload.FileUploadBase to the package
> org.apache.tomcat.util.http.fileupload.impl .
>
> Since some of those classes were public in FileUploadBase, they might have
> been used somewhere in other projects. Our Application for example uses
> org.apache.tomcat.util.http.fileupload.FileUploadBase.
> FileSizeLimitExceededException .
>
> This now ends in the fact, that compiled code will not run with newer 8.5.x
> Versions of Tomcat. As long as such projects or libraries are not fixed,
> customers cannot upgrade to newest Tomcat versions which will eventually end
> up in tomcat installations with known security issues.
That's a conceptual flaw I highly dislike:
> commit 218ea934fc71a0948c1b2e313e9cf20dede2cc23
> Author: Mark Thomas <ma...@apache.org>
> Date: 2019-12-06 16:30:48 +0100
>
> Merge in FileUpload changes to 2317552 (2019-12-06, 2.0-SNAPSHOT)
Relying on snapshots for GA versions is a no-go.
The other problem you see actually is that there is not enough abstraction
here. You rely on reshaded classes (exceptions) which should not be public (at
all). All should be hidden behind the Servlet API. Which is in Tomcat.
Do you
org.apache.tomcat.util.http.fileupload.FileUploadBase.FileSizeLimitExceededException
from #getCause()?
--
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 64290] Regression: The commit from December 06 2019 on
FileUploadBase introduced an incompatible API change
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64290
--- Comment #7 from Markus Schlegel <sc...@gmail.com> ---
(In reply to Mark Thomas from comment #5)
> Tomcat is going to continue to track HEAD for the various Apache Commons
> projects for which a local fork is maintained.
>
> Generally, a class being public doesn't make it part of Tomcat's public API.
> Tomcat's public API is defined in the RELEASE-NOTES file at the root of both
> the source and binary distributions. That said, we try to provide a lot more
> stability than the guarantees in that file.
Ok, thanks for the hint about the RELEASE-NOTES. I did not wirte the respective
code myself. It was a collegue, which I am sure of will never have read the
RELEASE-NOTES.
At times when we all were visiting the project's download page to get the
libraries source or binaries, such a RELEASE-NOTES might made sense.
Today, I guess 99% of the projects use some dependency-mgmt like maven or in
our case ivy, where a release-notes might not be read anymore. When a developer
decides to use some class, he will rely on the Javadoc of it.
In contrast to the releasenotes (which almost never change), the changelog is
THE SOURCE to find out if a new release fits into the project. Unfortunately,
that API change was not mentioned there.
> I can see why applications might be interested in the specific exception
> thrown. There are (ugly) ways to write code to handle both variations of
> class structure but in this instance I think it would be reasonable if we
> added back inner classes (deprecated and marked for removal in Tomcat 10) on
> an "as required" basis and have then simply extend the new classes.
>
> Markus, I'm assuming that the only class you need is
> FileUploadBase.FileSizeLimitExceededException
Yes, that's true. But as I stated in my previous comment, if this problem is
isolated to our specific usage only and if you think that this kind of class
use is "unique", then it makes no sense to just re-add that specific
innerclass, since we are now able to fix it in our maintenance code. But if
others might have used the inner classes in a similar way, then you should
consider re-adding all of them (extending the new ones of course).
--
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