Skip to content

Upgrade JQuery to 3.5.1 through webjars, for easier Maven version bumps #1972

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

Closed
wants to merge 2 commits into from

Conversation

timtebeek
Copy link
Contributor

Summary

Upgrade JQuery to 3.5.1 through webjars, for easier Maven version bumps

Details

Replace packaged JQuery-3.4.1.min.js with a webjars dependency, which can be bumped using the Maven version bump script. That way it should be easier to upgrade to newer versions without code, html and test changes.

Motivation and Context

Fixes #1971
After similar issues previously:
#1791
#1786
#1759

How Has This Been Tested?

Unit tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@timtebeek
Copy link
Contributor Author

@aikebah could you have a look if this resolves your OWASP issues, while still keeping the TimelineFormatter functional?

@timtebeek timtebeek requested a review from mpkorstanje May 12, 2020 20:09
@timtebeek
Copy link
Contributor Author

Now this does introduce two dependencies into cucumber-core(!) @mpkorstanje , which might be somewhat undesirable. I'll leave that up to decide whether this approach is acceptable.

</dependency>
<dependency>
<groupId>org.webjars</groupId>
<artifactId>webjars-locator-core</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might conflict with other applications that use this as a production dependency.

You'll have to shade both.

@aikebah
Copy link

aikebah commented May 12, 2020

@aikebah could you have a look if this resolves your OWASP issues, while still keeping the TimelineFormatter functional?

@timtebeek This resolves the OWASP dependency-check issue (as it would no longer find the JQuery 3.4.1 script in the jar and the dependency for jquery-3.5.1 has a JQuery script that has -for now- no known vulnerabilities associated).

But I can't say what it would do with respect to the timelineformatter. I just happened to have a maintenance task for an internal test-utility-library that defines some common Gherkin step definitions) and ran into the JQuery vulnerability being flagged in our build, which by default runs OWASP DependencyCheck as part of maven's verify phase.

I did an aggregate scan on all of Cucumber-JVM (source-branch of your PR) and see some other libs (that could use upgrades (as well as some false positives). I'll sift out the FPs (spotted some at first read, need further checks on others) and will file my findings as separate issue(s).

@mpkorstanje @timtebeek Would you prefer an issue per vulnerable lib, or an overall list of the vulnerable library versions that the Dependency-Check maven plugin was able to spot for the entire cucumber-jvm project tree?

As a side-note, do you know the dependabot preview? It would assist you keeping libraries up-to-date by filing per-lib PRs when it spots updates - for example this jackson databind upgrade PR in the DependencyCheck project.

@@ -38,7 +39,6 @@
"/io/cucumber/core/plugin/timeline/index.html",
"/io/cucumber/core/plugin/timeline/formatter.js",
"/io/cucumber/core/plugin/timeline/report.css",
"/io/cucumber/core/plugin/timeline/jquery-3.4.1.min.js",
Copy link
Contributor

@mpkorstanje mpkorstanje May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken, chosen.jquery (a few lines down) is a jquery plugin. That probably have to be updated as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chosen.jquery is indeed a jquery plugin, but there's no update for it. It's at its latest release (which was a securityfix) released in June 2018. Nowadays the project marks itself as deprecated awaiting decisions on their future direction.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented May 13, 2020

@aikebah much appreciated but to avoid shuffling reports around I ran the check myself.

mvn org.owasp:dependency-check-maven:aggregate -P-examples

edit:

mvn org.owasp:dependency-check-maven:aggregate -P-examples -DskipProvidedScope=true

After d840161 there weren't any actionable items left - jQuery aside. Either there is no new non-major version of the non-transitive dependency and/or the dependency is in the provided scope.

But please feel free to out any items you do find actionable. I'll take them as a single list.

As a side-note, do you know the dependabot preview? It would assist you keeping libraries up-to-date by filing per-lib PRs when it spots updates

I'm aware of it but not convinced it benefits Cucumber too much.

Upgrading dependencies may have implications for semantic versioning and needs to be paired up with the right release. This does not combine well with a strategy where we immediately upgrade dependencies. Instead upgrading dependencies is included as part of the release process. Being mostly automated through a make command it is not too much of a burden.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented May 14, 2020

@timtebeek the webjars-locator-core dependency is a bit heavy. Just jackson alone will result in frequent owasp findings. And without the locator I think it will be easier to stick with just using the files for now.

$ mvn dependency:tree
[INFO] +- org.webjars:jquery:jar:3.5.1:compile
[INFO] +- org.webjars:webjars-locator-core:jar:0.41:compile
[INFO] |  +- org.slf4j:slf4j-api:jar:1.7.7:compile
[INFO] |  +- io.github.classgraph:classgraph:jar:4.8.44:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.9.8:compile
[INFO] |  \- org.webjars.npm:angular__http:jar:2.4.10:compile

edit: Really appreciate the sentiment though!

@mpkorstanje mpkorstanje deleted the issue-1971-jquery-via-webjars branch May 14, 2020 18:40
@timtebeek
Copy link
Contributor Author

Agreed & understandable.. was just a nice experiment to try out this in a non-web setting. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulnerable version of jquery packaged in cucumber-core
3 participants