Hi Robert, Thanks for clarifying that requiring those 3 additional bundles was intentional. I've confirmed that the starter project builds ok and all the tests are passing after adding those to the distribution.
I don't know all the history, but I just thought it was a bit odd that a private copy of some of the dependencies are embedded in the xss bundle and others are not. In other words, is the XSS bundle trying to be a self-contained uber bundle, a slim bundle with the dependencies provided externally, or somewhere in between to workaround some problems? The 2.2.20 version has 6.831.549 bytes, while > the new one has 4.007.522 bytes. And the good part is that these > dependencies are now bundles, so they can be updated without needing to > cut a new Sling XSS update. > A smaller bundle is indeed an improvement. Though, ~4MB still seems a bit large for what this is doing. This probably wasn't a primary goal for this change, but was there consideration of reducing those duplicate private classes in the XSS bundle? Looks like there are private copies of google-guava, batik, various commons-* stuff and a few others embedded there. Regards, -Eric On Fri, Sep 23, 2022 at 9:22 AM Robert Munteanu <romb...@apache.org> wrote: > Hi Eric, > > On Thu, 2022-09-22 at 11:18 -0700, Eric Norman wrote: > > Can someone provide any details about what new dependencies this > > version of > > XSS API is expected to require? > > The main change is that we have moved from the AntiSamy library to the > OWASP Java HTML Sanitizer. More details and discussion at [1] and [2]. > > We keep using the same format for the configuration file, which means > that we have to parse the XML somehow. The library that was selected > was Jackson - it's widely used, has a good community, and permissively > licensed. > > The woodstox dependency comes from Jackson. Some time ago I had pushed > a commit that updated the Sling Starter to have the additional needed > dependencies [3]. > > That may look like a downside, but keep in mind that the "old" Sling > XSS bundle embedded many more dependencies, including XML parsers and > was very hard to evolve. The 2.2.20 version has 6.831.549 bytes, while > the new one has 4.007.522 bytes. And the good part is that these > dependencies are now bundles, so they can be updated without needing to > cut a new Sling XSS update. > > > > > I couldn't plug the SNAPSHOT build into the starter as it would not > > build > > due to unresolved dependencies on packages from jackson-dataformat- > > xml > > (and woodstox?). > > > > Is there any chance that the code could be refactored to not require > > those > > additional dependencies? > > Jackson is pretty core to the refactoring. I'm not saying it's > impossible, but someone would have to do the legwork for that. > > Looking at the Jackon pom it seems that we don't _have to_ use > Woodstox, but the code does not seem to work without it right now. > > Thanks, > Robert > > > [1]: https://issues.apache.org/jira/browse/SLING-7231 > [2]: https://github.com/apache/sling-org-apache-sling-xss/pull/28 > [3]: > > https://github.com/rombert/sling-org-apache-sling-starter/commit/0372ef37203086dc1b930bffe86f0ee1a81ceaf9 > [4]: > > https://github.com/FasterXML/jackson-dataformat-xml/blob/42f584f05e1e8566f79dc7251d6cb826f2505c13/pom.xml#L78-L81 > > > > > Regards, > > -Eric > > > > On Wed, Sep 21, 2022 at 5:44 AM Robert Munteanu <romb...@apache.org> > > wrote: > > > > > Hi, > > > > > > I just merged the XSS API changes for removing the AntiSamy library > > > and > > > using the OWASP Java HTML Sanitizer instead [1], [2]. (Thanks, > > > Tatyana!) > > > > > > As it's a big change and, even though it's coming with lots of > > > tests, I > > > would like to wait a bit before releasing. > > > > > > I would like to start the release vote in a week from now, on > > > 28.09. > > > > > > Please let me know if you discover any release blockers before. > > > > > > Thanks, > > > Robert > > > > > > [1]: https://issues.apache.org/jira/browse/SLING-7231 > > > [2]: https://github.com/apache/sling-org-apache-sling-xss/pull/28 > > > > >
