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

SLING-12276 - Update to java-html-sanitizer 20240325.1 #42

Merged
merged 5 commits into from Apr 12, 2024

Conversation

rombert
Copy link
Contributor

@rombert rombert commented Mar 26, 2024

  • remove shade plugin configuration
  • stop embedding guava classes
  • rework our overrides to no longer use Guava

Bundle size is down from 4.1 to 1.9 MB

- remove shade plugin configuration
- stop embedding guava classes
- rework our overrides to no longer use Guava

Bundle size is down from 4.1 to 1.9 MB
@rombert
Copy link
Contributor Author

rombert commented Mar 26, 2024

After this PR the bundle will no longer resolve in the current Sling Starter

[ERROR] [bundle-packages] org.apache.sling:org.apache.sling.xss:2.4.1-SNAPSHOT: Bundle is importing package org.owasp.shim with start order 20 but no bundle is exporting these for that start order.

Also inline the new shim classes in the resulting jar.
@rombert rombert marked this pull request as ready for review March 26, 2024 10:28
@rombert
Copy link
Contributor Author

rombert commented Mar 26, 2024

After this PR the bundle will no longer resolve in the current Sling Starter

[ERROR] [bundle-packages] org.apache.sling:org.apache.sling.xss:2.4.1-SNAPSHOT: Bundle is importing package org.owasp.shim with start order 20 but no bundle is exporting these for that start order.

Fixed with a6efb27

@kwin
Copy link
Member

kwin commented Mar 26, 2024

java-html-sanitizer 20240325.1 contains Java 10 bytecode (https://github.com/OWASP/java-html-sanitizer/blob/43089899bae8fae0cb0016c5700beace7ddd26f0/owasp-java-html-sanitizer/pom.xml#L90), so we should target Java10+ as well with our wrapper bundle.

Alternatively we can use https://github.com/OWASP/java-html-sanitizer/blob/43089899bae8fae0cb0016c5700beace7ddd26f0/java8-shim/pom.xml instead...

@rombert
Copy link
Contributor Author

rombert commented Mar 26, 2024

@kwin - my reading of the release notes is that this release still supports Java 8. The Jenkins build with Java 8 also passes - https://ci-builds.apache.org/blue/organizations/jenkins/Sling%2Fmodules%2Fsling-org-apache-sling-xss/detail/PR-42/1/pipeline/122 .

Fix Java 8 build after inlining the shim packages.
Fix bnd warnings (the shim packages don't overlap anyway).
@rombert
Copy link
Contributor Author

rombert commented Mar 26, 2024

I can't actually run the latest Sling Starter with Java 8, but the analysers are not happy

[ERROR] [requirements-capabilities] org.apache.sling:org.apache.sling.xss:2.4.1-SNAPSHOT: Artifact org.apache.sling:org.apache.sling.xss:2.4.1-SNAPSHOT requires [org.apache.sling.xss/2.4.1.SNAPSHOT] osgi.ee; filter:="(&(osgi.ee=JavaSE)(version=10))" in start level 20 but no artifact is providing a matching capability in this start level.

This is probably because of the Java 10 shim; we should overide the requirement that bnd generates in this case.

@kwin
Copy link
Member

kwin commented Mar 26, 2024

Force the osgi.ee requirement back to Java 8
Copy link

sonarcloud bot commented Mar 27, 2024

@stefanseifert
Copy link
Member

lgtm from sling-mock perspective

@rombert
Copy link
Contributor Author

rombert commented Apr 11, 2024

@raducotescu - any comments? I plan to merge tomorrow.

@rombert rombert merged commit 7b44bef into master Apr 12, 2024
2 checks passed
@rombert rombert deleted the issue/SLING-12276 branch April 12, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants