Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove of commons-digester from core #5320

Merged
merged 2 commits into from
Jun 3, 2021

Conversation

olamy
Copy link
Member

@olamy olamy commented Mar 1, 2021

See JENKINS-65161.

Proposed changelog entries

  • Remove commons-digester wrapper from core and the dependencies entries as well

Proposed upgrade guidelines

With this change commons-digester will not be anymore provided by core.
Some plugins were using it and pull requests has been made to fix the removal (see the list in this PR).
If you need use commons-digester from a plugin, you can look at this PR (jenkinsci/plasticscm-plugin#40) to understand the change.
Or use this code snippet:

    import org.apache.commons.digester3.Digester;
    ....
    public static Digester createDigester(boolean secure) throws SAXException {
        Digester digester = new Digester();
        if (secure) {
            digester.setXIncludeAware(false);
            try {
                digester.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
                digester.setFeature("http://xml.org/sax/features/external-general-entities", false);
                digester.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
                digester.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
            } catch (ParserConfigurationException ex) {
                throw new SAXException("Failed to securely configure xml digester parser", ex);
            }
        }
        return digester;
    }

Corresponding plugin PRs

These plugins were identified using the following approaches:

release status

Legend:
✔️ : Fixed or no need to fix
🛑 : It's not going to be fixed (no maintainer mainly)
❗ : Waiting for something (merge or release)

Status Plugin Fix PR and/or status Number of OSS Installations
✔️ https://github.com/jenkinsci/jelly only used by RSS Reader and core is producing RSS not reading it infinity and beyond
✔️ https://github.com/jenkinsci/jenkins-test-harness Tests only
✔️ 2.14.1 subversion jenkinsci/subversion-plugin#254 170k
✔️ 2.19 cvs jenkinsci/cvs-plugin#55 39k
✔️ 0.3.0 maven-info jenkinsci/maven-info-plugin#9 (last release 7 years ago) 5502
✔️ 4.12.0 clover jenkinsci/clover-plugin#24 3521
✔️ 1.31 emma jenkinsci/emma-plugin#11 (last real activity 6 years ago) 3216
✔️ 0.6 cloverphp jenkinsci/cloverphp-plugin#10 (last activity 6 years ago...) 2,801
✔️ 1.6.7 clearcase jenkinsci/clearcase-plugin#41 2094
✔️ 2.4.0 teamconcert jenkinsci/teamconcert-plugin#20 (last activity/release early 2020) 1640
❗ awaiting merge vs-code-metrics jenkinsci/vs-code-metrics-plugin#5 (last activity/release 2014) 1435
🛑 (no maintainer) BlameSubversion jenkinsci/BlameSubversion-plugin#5 (last activity 8 years ago...) 878
🛑 (no maintainer) javatest-report jenkinsci/javatest-report-plugin#4 (last real activity 6 years ago) 440
✔️ 3.6 plasticscm-plugin jenkinsci/plasticscm-plugin#40 (last release late 2020, recent activity) 284
✔️ 1.7.3 clearcase-ucm jenkinsci/clearcase-ucm-plugin#5 (last release 2016) 266
✔️ 0.17 vectorcast-coverage jenkinsci/vectorcast-coverage-plugin#4 (last release in 2021) 206
✔️ 2.3.5 zos-connector jenkinsci/zos-connector-plugin#13 (recent activity in 2020) 173
🛑 (no maintainer) vss jenkinsci/vss-plugin#8 (last release/activity 2011) 168
✔️ 1.10 genexus jenkinsci/genexus-plugin#15 (activity Sept 2020 & release in April 2020) 149
✔️ 0.9.1 dimensionsscm jenkinsci/dimensionsscm-plugin#21 113
🛑 (no maintainer) synergy jenkinsci/synergy_scm-plugin#17) (last activity 6 years ago) 96
🛑 (no maintainer, needed repo missed) config-rotator jenkinsci/config-rotator-plugin#3 (last activity 4 years ago) 🚨 need help from Praqma, as https://code.praqma.net/repo/maven/ no longer exists 62
🛑 (no maintainer) harvest jenkinsci/harvest-plugin#5 (last activity 6 years ago) 49
✔️ 0.16 plasticscm-mergebot jenkinsci/plasticscm-mergebot-plugin#3 (last active/release late 2019) 55
🛑 (no maintainer) cmvc jenkinsci/cmvc-plugin#3 (last activity 9 years ago...) 18

Plugins that got suspended

Never released

Submitter checklist

  • (If applicable) Jira issue is well described
  • Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

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

  • There are at least 2 approvals for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title and/or Proposed changelog entries are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • 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).

@olamy olamy added dependencies Pull requests that update a dependency file java Pull requests that update Java code labels Mar 1, 2021
@olamy olamy marked this pull request as draft March 1, 2021 06:03
@olamy
Copy link
Member Author

olamy commented Mar 1, 2021

Still a draft as this need some discussions.
This upgrade need some package changes in commons-digester library but it should be covered as the hudson.util.Digester2 is a wrapper on a top of it.
But I cannot see any usage in core of the class hudson.util.Digester2.
why not simply remove it?

@rsandell
Copy link
Member

rsandell commented Mar 1, 2021

Have you checked the usage in plugins? I have a hunch that I might have used it somewhere a long time ago :)

@daniel-beck
Copy link
Member

why not simply remove it?

Used a lot in plugins, which is why it got a security hardening recently. Please use https://github.com/jenkins-infra/usage-in-plugins to identify uses of APIs you plan to change.

@jglick
Copy link
Member

jglick commented Mar 3, 2021

I would rather suggest deprecating Digester2 and maybe detaching it to a split plugin, unless we can kill all plugin references.

@olamy
Copy link
Member Author

olamy commented Mar 9, 2021

Used in 14 plugins:

@daniel-beck
Copy link
Member

daniel-beck commented Mar 30, 2021

Digester does not by default prevent XXE vulnerabilities. The recent change in core to Digester2 does, that's the hardening I was referring to in an earlier comment. So unless a plugin already specifically opts out of bad defaults (like CVS), those linked PRs create XXE vulnerabilities.

@olamy
Copy link
Member Author

olamy commented Mar 30, 2021

ok I will fix the PRs but we should remove this from core as we do not use it.

@batmat batmat merged commit 9abd082 into jenkinsci:master Jun 3, 2021
@batmat
Copy link
Member

batmat commented Jun 3, 2021

@bitwiseman the table just got updated with latest status a few seconds ago, can you please file a PR to update the JEP-231 with impacted plugins?

Thank you!

@trivalik
Copy link

@timja
Copy link
Member

timja commented Jun 15, 2021

The jenkinsci/tfs-plugin is not suspended see issues.jenkins-ci.org/browse/INFRA-2751

@trivalik it is until:

In regards to security issues, we just announced more problems with the plugin in https://www.jenkins.io/security/advisory/2021-03-30/ and we require those be addressed before the plugin distribution is restored. I'd be happy to be involved in review to ensure those are properly resolved.

Likely not terribly hard to do, just needs someone to step up and do it

daniel-beck pushed a commit to jenkins-infra/update-center2 that referenced this pull request Jun 21, 2021
jlamasrios added a commit to jlamasrios/update-center2 that referenced this pull request Jun 23, 2021
Although included in the primary list of candidates for deprecation due to jenkinsci/jenkins#5320, the plugin was fixed on its 1.10 release on June 3, and removed from the list (see https://github.com/bitwiseman/jep/tree/00c184f97f40c5f79f8f83074c78dd4fb1e1ac43/jep/231)
The plugin does have mantainers and is not deprecated
jlamasrios added a commit to jlamasrios/update-center2 that referenced this pull request Jun 23, 2021
Although included in the primary list of candidates for deprecation due to jenkinsci/jenkins#5320, the plugin was fixed on its 1.10 release on June 3, and removed from the list (see https://github.com/bitwiseman/jep/tree/00c184f97f40c5f79f8f83074c78dd4fb1e1ac43/jep/231)
The plugin does have mantainers and is not deprecated
batmat added a commit to batmat/jep that referenced this pull request Jul 7, 2021
@trivalik
Copy link

trivalik commented Jul 7, 2021

@timja Are these related to this class https://github.com/jenkinsci/tfs-plugin/blob/master/tfs/src/main/java/hudson/plugins/tfs/model/Server.java?

I provide a pull request for SECURITY-1506 / CVE-2020-2249 jenkinsci/tfs-plugin#244 but nobody is review nor merging it.

@timja
Copy link
Member

timja commented Jul 7, 2021

responded in the tfs plugin PR

testcocoon pushed a commit to testcocoon/cocoemma-plugin that referenced this pull request Sep 17, 2021
This is related to the following changes of Jenkins:
jenkinsci/jenkins#5320
alecharp added a commit to alecharp/workflow-cps-global-lib-plugin that referenced this pull request Nov 4, 2021
@basil
Copy link
Member

basil commented Dec 19, 2021

Causes the test harness to stop compiling against recent cores: ExtractChangeLogParser#parse.

@jglick
Copy link
Member

jglick commented Dec 20, 2021

Should be easy enough to fix in the usual way (jenkinsci/subversion-plugin#259).

@basil
Copy link
Member

basil commented Dec 20, 2021

@olamy FYI

@basil
Copy link
Member

basil commented Feb 5, 2022

Causes the test harness to stop compiling against recent cores: ExtractChangeLogParser#parse.

@olamy Do you intend to fix this regression?

@olamy
Copy link
Member Author

olamy commented Feb 6, 2022

@basil yup on it.

Adakar pushed a commit to jenkinsci/cocoemma-plugin that referenced this pull request Apr 11, 2022
Adakar added a commit to jenkinsci/cocoemma-plugin that referenced this pull request Apr 11, 2022
Adakar added a commit to jenkinsci/cocoemma-plugin that referenced this pull request Apr 11, 2022
* Update minimum Jenkins version, BOM and parent POM

Jenkins minimum version  2.204.2 -> 2.303.1
Parent POM 4.00 -> 4.38
BOM 2.204.x -> 2.303.x

* Update pom dependencies

* Update plugin to use commons.digester3

The change is based on the jenkinsci/jenkins#5320 PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file developer Changes which impact plugin developers java Pull requests that update Java code ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback removed This PR removes a feature or a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.