You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "enapps-enorman (via GitHub)" <gi...@apache.org> on 2026/05/21 18:57:36 UTC
[PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
enapps-enorman opened a new pull request, #505:
URL: https://github.com/apache/felix-dev/pull/505
Jetty does not need the Apache commons-fileupload library. Modern versions of Jetty support the Java Servlet Specification (3.0 and newer), which includes built-in, native APIs to parse multipart/form-data requests.
Refactoring to use the standard servlet apis:
1. remove the commons-fileupload dependency
2. Change ServletRequestMultipartWrapper to delegate the multipart request handling to the jetty servlet container
3. Other refactoring to replace the org.apache.commons.fileupload references with the servlet Part equivalent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4617479091
@paulrutter @cziegeler I've merged the latest changes and resolved the merge conflicts. If there are no other concerns, then please merge on my behalf since I don't have rights to merge in this repository.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4560834840
@enapps-enorman Thanks! If the same set of tests failed already before this change, then we should track those separately. But we need to make sure to pass all of them again, initially we did.
Its good that we can get rid of commons-fileupload, I tried a similar approach years ago with jetty 9 and couldnt succeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4611029164
@enapps-enorman can you sync with `master` and resolve conflicts. We just moved to Jetty 12.1.10`.
@cziegeler will be merge this one once resolved?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4571444288
Our implementation had a bug and was wrongly setting the EXCEPTION_TYPE attribute to the class name, not the class. This bug was reflected in the TCK. We fixed the implementation and the TCK got fixed after the 8.1.0 release (https://github.com/osgi/osgi/commit/f38f8b32a720ab9a513a20d67240d7f4c8b863ed)
This explains the one error for the servlet test cases and one out of the ten for the whiteboard test cases.
I'll investigate the other 9 as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4515090088
I suggest to run the OSGi TCK for the http service, http whiteboard and servlet whiteboard on the proposed change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4547257508
> Yes, it is that public git repo, you need to run the test cases from the R8 cmpn release:
@cziegeler Ok. Unfortunately running the test cases off those branches fails for all felix.http.jetty versions above 5.x with what looks like some resolution problems with missing slf4j packages? I'm not familiar with this codebase at all, so if you have some hints on what and where is the correct places to make adjustments to make the felix.http.jetty 5.x versions function I would appreciate it. For example, felix.http.jetty 4.2.32 is the last version that seems to work, but 5.0.0 does not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4533980076
Yes, it is that public git repo, you need to run the test cases from the R8 cmpn release:
- https://github.com/osgi/osgi/tree/r8-cmpn-final/org.osgi.test.cases.http
- https://github.com/osgi/osgi/tree/r8-cmpn-final/org.osgi.test.cases.http.whiteboard
- https://github.com/osgi/osgi/tree/r8-cmpn-final/org.osgi.test.cases.http.whiteboard.secure
and R8.1 cmpn release:
- https://github.com/osgi/osgi/tree/8.1.0.cmpn/org.osgi.test.cases.servlet
- https://github.com/osgi/osgi/tree/8.1.0.cmpn/org.osgi.test.cases.servlet.secure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4612029602
yes, I think we are good to go (once conflicts are resolved)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4559671032
> Its too long ago to remember the details
Ok. I got some hints from copilot and I think I figured out what to change to get the tests to run. Below is a summary of the results. NOTE: There are some tests failing but from what I can tell it is the same set of tests that also fail with http.jetty:5.2.2. I can try to take a closer look at the failures when I get some free time. But maybe those should be tracked separately from this one.
With these changes: https://github.com/osgi/osgi/compare/r8-cmpn-final...enapps-enorman:osgi:r8-cmpn-final-jetty5.2.3?expand=1
r8-cmpn-final/org.osgi.test.cases.http
Test run finished after 504 ms
[ 5 containers found ]
[ 0 containers skipped ]
[ 5 containers started ]
[ 0 containers aborted ]
[ 5 containers successful ]
[ 0 containers failed ]
[ 29 tests found ]
[ 0 tests skipped ]
[ 29 tests started ]
[ 0 tests aborted ]
[ 29 tests successful ]
[ 0 tests failed ]
r8-cmpn-final/org.osgi.test.cases.http.whiteboard
Test run finished after 6583 ms
[ 12 containers found ]
[ 0 containers skipped ]
[ 12 containers started ]
[ 0 containers aborted ]
[ 12 containers successful ]
[ 0 containers failed ]
[ 159 tests found ]
[ 0 tests skipped ]
[ 159 tests started ]
[ 0 tests aborted ]
[ 149 tests successful ]
[ 10 tests failed ]
r8-cmpn-final/org.osgi.test.cases.http.whiteboard.secure
Test run finished after 2291 ms
[ 4 containers found ]
[ 0 containers skipped ]
[ 4 containers started ]
[ 0 containers aborted ]
[ 4 containers successful ]
[ 0 containers failed ]
[ 6 tests found ]
[ 0 tests skipped ]
[ 6 tests started ]
[ 0 tests aborted ]
[ 5 tests successful ]
[ 1 tests failed ]
With these changes: https://github.com/osgi/osgi/compare/8.1.0.cmpn...enapps-enorman:osgi:8.1.0.cmpn-jetty5.2.3?expand=1
8.1.0.cmpn/org.osgi.test.cases.servlet
Test run finished after 6620 ms
[ 12 containers found ]
[ 0 containers skipped ]
[ 12 containers started ]
[ 0 containers aborted ]
[ 12 containers successful ]
[ 0 containers failed ]
[ 147 tests found ]
[ 0 tests skipped ]
[ 147 tests started ]
[ 0 tests aborted ]
[ 146 tests successful ]
[ 1 tests failed ]
8.1.0.cmpn/org.osgi.test.cases.servlet.secure
Test run finished after 4325 ms
[ 4 containers found ]
[ 0 containers skipped ]
[ 4 containers started ]
[ 0 containers aborted ]
[ 4 containers successful ]
[ 0 containers failed ]
[ 6 tests found ]
[ 0 tests skipped ]
[ 6 tests started ]
[ 0 tests aborted ]
[ 6 tests successful ]
[ 0 tests failed ]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4512389106
@enapps-enorman I like the approach, it's just another dependency we can do without nowadays. I'm fine with removing and delegation to Jetty instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4551880928
Its too long ago to remember the details, but I think it required to add some additional bundles in some of these library files. If the main branch works for you, you could also just try to move the test sub projects from those branches into the main branch and execute it there. I think thats what I did last time...
We also have https://felix.apache.org/documentation/development/using-the-osgi-compliance-tests.html - but thats completely outdated and doesnt help at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4512117988
I believe the now unused ServletHolder#getMultipartSecurityContext and related code could also be removed since the only usage was in the now removed multipart handling code.
But I thought I would get some feedback on the general approach before doing that extra cleanup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #505:
URL: https://github.com/apache/felix-dev/pull/505#issuecomment-4530047996
> I suggest to run the OSGi TCK
For clarification, is that referring to the test cases published at https://github.com/osgi/osgi or something private I don't have access to? If it is the former then I tried running it against jetty-5.2.3-SNAPSHOT and I didn't get any test failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6829 Remove http.jetty dependency on commons-fileupload [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter merged PR #505:
URL: https://github.com/apache/felix-dev/pull/505
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org