[JENKINS-73170] Upgrade Commons FileUpload from 1.5 to 2.0.0-M2

[basil]

JENKINS-73170 Upgrade Commons FileUpload from 1.5 to 2.0.0-M2

Context

JENKINS-73169 describes the long-term goal of removing Commons FileUpload from Stapler and Jenkins core, decreasing API surface area and maintenance burden by eliminating an unneeded library.

This long-term strategy carries a certain amount of risk and labor:

• New APIs need to be designed and validated across a variety of consumers (both core consumers and plugin consumers), which is more laborious than a textual package/class rename operation.
• New implementations need to be written and tested, ensuring that existing behavior remains the same and no new security issues are introduced. Given the general lack of realistic automated testing for file uploading, there is a high risk of behavioral regression. Even worse is the potential for a security regression that a new implementation brings. Testing all of these scenarios thoroughly will require a large amount of manual testing and is far more laborious than staying with a battle-tested implementation with known (and tested) security modalities.
• Implementing this change throughout the ecosystem would require the new APIs to ship in a weekly/LTS release, for plugins to adopt that LTS release, and then for those plugins to be adapted to using the new APIs. The list is:

  plain-credentials|278830
  subversion|86698
  uno-choice|37949
  google-oauth-plugin|12220
  m2release|9733
  scriptler|6767
  sidebar-link|6678
  file-parameters|5507
  xcode-plugin|4506
  kpp-management-plugin|2262
  miniorange-saml-sp|2099
  hp-application-automation-tools-plugin|1611
  custom-folder-icon|1158
  promoted-builds-simple|1038
  custom-job-icon|712
  jclouds-jenkins|583
  patch-parameter|559
  svn-partial-release-mgr|517
  htmlresource|515
  BlameSubversion|405
  deployer-framework|271
  avatar|214
  ios-device-connector|125
  pom2config|90
  recipe|51
  clif-performance-testing|22
  

  Our experience has shown that it takes many months for such changes to propagate through the ecosystem and to be adopted by plugins. Upgrading a plugin's baseline, patching it to use new APIs, and getting the result merged and released can be a time-consuming exercise, if not a frustrating one—especially for the long tail of plugins.

Problem

We have a desire to upgrade Spring Security to a recent version in a relatively timely fashion. Recent versions of Spring Security require Java EE 9 or later, while Commons FileUpload 1.x only supports Java EE 8 and earlier, implying in the long term the completion of the abovementioned project as a prerequisite to moving to Java EE 9 or later (or, in the short term, creating a custom fork of Commons FileUpload 1.x with the javax imports changed to jakarta imports—a high-maintenance prospect that would also regress our previous efforts to unfork this library). Yet this long-term solution is laborious and carries a high risk of regression (both functionally and security-wise); moreover, file uploading is not a central feature of the software, and focusing a huge amount of effort on it seems out of place relative to the value to end users it will provide—certainly a higher priority would be to focus on the actual Jakarta EE 9 migration itself, which brings with it the tangible benefits of security updates for Spring Security and thus a clear benefit to Jenkins users.

Solution

A medium-term compromise is possible via Commons FileUpload 2.x, which supports both javax and jakarta imports. Consider that Stapler and Jenkins core provide a number of APIs that work with Commons FileUpload FileItem objects, in contrast to the low-level DiskFileItem objects and the classes that produce them (like DiskFileItemFactory). These APIs somewhat shelter their consumers from low-level implementation details, like the instantiation of a factory against a particular servlet API. The key observation is that the vast majority of plugins (all open-source plugins with more than 3,000 installations, and all but two proprietary plugins) consume these higher-level Jenkins APIs, which still tie us to Commons FileUpload, but not necessarily to low-level implementation details like DiskFileItem; moreover, any cases I found of plugins that directly consume the low-level APIs can be trivially converted to the higher-level Stapler APIs. Unfortunately, the package name has changed between 1.x and 2.x; however, since the FileItem interface is very similar between 1.x and 2.x, and since it is an interface, we can bridge between the two versions somewhat transparently.

The implications are as follows: once a handful of plugins are migrated from direct usages of DiskFileItem to the simpler FileItem wrappers provided by Stapler (which is a trivial and low-risk change, and moreover only needs to be done for 2-4 plugins in the short term), we can upgrade core to FileUpload 2.x (with both javax and jakarta support) maintaining full plugin compatibility (in both production and build/test) with any plugins that consume the 1.x APIs via FileItem (and not through the low-level DiskFileItem). These existing plugins, which link against FileUpload 1.x, will call into Stapler to get their FileItem, which will be a 2.x object wrapped in a 1.x compatibility object, and any methods they call on that 1.x compatibility object will vector back into the 2.x methods at runtime.

In other words, core will be fully running on FileUpload 2.x, with both javax and jakarta support, unblocking the Java EE 9 migration project. Even better, this can be achieved through a fraction of the labor and the risk of the long-term strategy. Commons FileUpload 2.x is largely a package/class rename release, but the code is basically the battle-tested implementation we have always relied on, so the risk of functional and security regression is very low. Rather than having to sweep through dozens of plugins, we'll have to sweep through a handful at most—and the results of that sweep can be released proactively, rather than having to wait for a new API to be available (perhaps even in LTS) in order to consume it. Finally, this gets file uploading out of the way for the time being so that Jenkins contributors can focus on the higher-priority task of dealing with the Jakarta EE 9 migration.

Some might object to the use of a milestone release of Commons FileUpload, and this is a fair point. Yet it is advertised on both the project's home page and their GitHub page; also, it is published to Maven Central (unlike a SNAPSHOT release). It would appear as if the authors are getting close to a release but simply looking for feedback on the API. The Jenkins project could provide a valuable service to the Apache Commons project by confirming that the API is working for our use case. Moreover, I suspect the Spring 5.3.x EOL will likely push the whole Java ecosystem toward Java EE 9+, and I suspect (or at least hope) that this will push Commons FileUpload 2.x toward GA this year. Nevertheless, since it is still in milestone status, I am not recommending that we go ahead and migrate plugins from FileUpload 1.x to 2.x APIs at this time. The 1.x compatibility methods are tried and true, and not subject to change, so it is prudent to remain on them at least until the GA release of 2.x. Moreover, going on a sweep of plugin consumers is exactly the kind of activity that we wanted to avoid doing as part of this exercise; and if we are going to do a sweep of plugin consumers, then we might as well simply implement the long-term solution—something which this medium-term solution in no way precludes, but simply postpones to a later date to focus on more pressing concerns. Finally, with the exception of the FileItem compatibility interface, this is a lateral move in API surface area (removal of 1.x and addition of 2.x) rather than an increase (addition of 2.x to the existing 1.x).

Looking at the bigger picture, shipping this immediately will cross out one major portion of the risk and labor associated with the Jakarta EE 9 upgrade—that which is associated with file uploading. Getting this out into weekly releases early will ensure that we have plenty of time to deal with any issues. The next major change after this will be to require Java 17, at which point we can switch to Jetty 12.x with Jakarta EE 8 and get another big wad of changes out into production. Then, the only remaining part of the problem will be to switch from Jetty 12 (EE 8) and FileUpload 2 (EE 8) to Jetty 12 (EE 9) and FileUpload 2 (EE 9), a much more bounded problem, whose risk will also be a lot lower, since some of its major prerequisites will have already been shipping for weeks earlier. In contrast, trying to ship a FileUpload change, a Jetty upgrade, and a Jakarta migration all in the same release sounds like a scope that is far too great to be practical.

Implementation

Usages in core and Stapler are migrated to 2.x. A full compatibility layer is provided for plugins that use the high-level FileItem API (but not the low-level DiskFileItem API). This compatibilty layer extends to both production as well as compilation/tests.

The compatibility chart is here (there are also two CloudBees plugins, and I will find someone to update those). The only open-source plugins with over 1,000 installations are miniorange-saml-sp and hp-application-automation-tools-plugin—those are the only two I plan to help with or mention in the release notes, but even those are installed on only 0.72% of controllers so I don't think they should block the integration of this PR into the weekly line (this is just too important a milestone and we are just on too tight of a timeline to wait).

Testing done

• Full PCT and ATH (both passing)
• Manual testing of file uploading in both core (via the upload plugin feature) and two plugins (Scriptler and Sidebar Link) using the compatibility methods
• Existing security-related unit tests still pass, as does the unit test I added recently for file upload persistence.

Proposed changelog entries

• Upgrade Commons FileUpload from 1.5 to 2.0.0-M2. Users of the miniorange-saml-sp plugin should upgrade to a compatible version in lockstep with upgrading Jenkins core.

Proposed upgrade guidelines

• Upgrade Commons FileUpload from 1.5 to 2.0.0-M2. Users of the miniorange-saml-sp plugin should upgrade to a compatible version in lockstep with upgrading Jenkins core.

Submitter checklist

Beta Give feedback

1. The Jira issue, if it exists, is well-described.
2. The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
3. There is automated testing or an explanation as to why this change has no tests.
4. New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
5. New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
6. New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
7. For dependency updates, there are links to external changelogs and, if possible, full differentials.
8. For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Beta Give feedback

1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
4. Proper changelog labels are set so that the changelog can be generated automatically.
5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[github-actions]

Please take a moment and address the merge conflicts of your pull request. Thanks!

[basil]

It would be nice if there was a BOM, but I didn't find one, nor did I find an RFE bug asking for one, so I included every package in this library in the Jenkins BOM for completeness.

[basil]

See the commentary in the Stapler PR for an explanation of how this works.

[basil]

This is basically only consumed by tests, and might even make more sense in the test harness, though I am not tackling that here. However, to retain compatibility at compile time and in tests, it has to retain the old API. I am providing a FileItemImpl2 that implements the new API for use by core itself and implement this in terms of that (through the delegate pattern) in order to reuse code.

[basil]

See the explanation for this in the Stapler PR.

[daniel-beck]

Reviewed wrt SECURITY-3030 and SECURITY-3073, seems fine.

[MarkEWaite]

Thanks!

[basil]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process.