Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-19 Thread Robert Munteanu
On Tue, 2019-11-19 at 11:13 +0100, Radu Cotescu wrote:
> some of the dependencies’ classes used
> APIs not available in the JRE, but provided by Sling and this made
> the build fail on Java 8

Ah, that was the missing piece for me. All clear now, thank you.

Robert



Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-19 Thread Radu Cotescu
Hi Konrad,

> On 19 Nov 2019, at 11:30, Konrad Windszus  wrote:
> 
> This is not exactly true. Maybe you can use the 
> https://bnd.bndtools.org/instructions/conditionalpackage.html 
>  
>  > instruction? 
> That can be used for embedding with bnd-maven-plugin.

TIL! It’s funny how the wording on that page completely avoids the word embed.

I’m not sure I want to touch the configs again, now that I finally made them 
work. I would probably just have to move the list of private packages that we 
have now into this -conditionalpackage instruction, but I’m afraid to do it 
given the complexity of this bundle in particular. Problem for future Radu or 
another Sling committer. :D

Thanks for the pointer, though!

Regards,
Radu



Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-19 Thread Konrad Windszus
Hi Radu,
regarding

> 
> the bnd-maven-plugin doesn’t provide an embed / inline feature. 
This is not exactly true. Maybe you can use the 
https://bnd.bndtools.org/instructions/conditionalpackage.html 
 instruction? 
That can be used for embedding with bnd-maven-plugin.
Thanks,
Konrad

Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-19 Thread Radu Cotescu
Hi Robert,

> On 19 Nov 2019, at 10:22, Robert Munteanu  wrote:
> 
> Sorry for the late reply. I'm not sure I follow. If you inline classes
> that require Java 11, doesn't the bundle effectively require Java 11?

We haven’t inlined classes that require Java 11 APIs.

Let me try to describe again what the issue was. Before switching to building 
the bundle with the bnd-maven-plugin, some of the project’s dependencies were 
embedded and inlined with the maven-bundle-plugin. The animal sniffer plugin’s 
job, though, was to make sure that the project’s classes don’t use APIs not 
available on the target JRE.

With the switch to the sling-bundle-parent, we don’t use the 
maven-bundle-plugin any more and the bnd-maven-plugin doesn’t provide an embed 
/ inline feature. So I delegated this to the maven-dependency-plugin for 
unpacking the “to embed” dependencies into ${project.build.outputDirectory} and 
the default maven-jar-plugin for packing. However, I was running the 
maven-dependency-plugin in the generate-resources phase. The sniffer plugin 
runs its checks in the process-classes phase by default. That led to the 
sniffer plugin checking also the dependencies’ classes (on Java 8 only; it’s 
disabled on Java 11 since the javac does the same checks better). However, like 
you noticed, some of the dependencies’ classes used APIs not available in the 
JRE, but provided by Sling and this made the build fail on Java 8. By default, 
the animal sniffer plugin doesn’t check dependencies. Even if the build failed 
with Java 8, the same module built with Java 11 will run without any issues on 
JRE 1.8, since that’s the target platform.

In version 2.1.16 I’ve corrected how the dependencies are inlined and, when 
building with Java 8, the animal sniffer plugin will only check the project’s 
source code, like before.

Cheers,
Radu




Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-19 Thread Robert Munteanu
On Fri, 2019-11-15 at 16:50 +0100, Radu Cotescu wrote:
> Hi Robert
> 
> > On 15 Nov 2019, at 16:40, Robert Munteanu 
> > wrote:
> > 
> > Hi Radu,
> > 
> > On Fri, 2019-11-15 at 16:36 +0100, Radu Cotescu wrote:
> > > Hi,
> > > 
> > > Builds on Java 8 fail [0] due to how I handled dependency
> > > inlining
> > > for SLING-8847 [1], since the sniffer plugin will also check the
> > > dependencies’ classes. Those classes should be inlined in the
> > > prepare-package phase instead of generate-resources.
> > > I was building locally with Java 11, hence why I didn’t see any
> > > issues before starting the release.
> > 
> > IIUC, the error is
> > 
> > [ERROR] /home/jenkins/jenkins-slave/workspace/ling-org-apache-
> > sling-
> > xss_master/target/classes/org/owasp/esapi/tags/BaseEncodeTag.class:
> > 29:
> > Undefined reference: void
> > javax.servlet.jsp.tagext.BodyTagSupport.()
> 
> Because I’ve inlined the dependencies’ classes in the generate-
> resources phase, the animal-sniffer plugin will consider those
> classes as part of the project. Apparently some ESAPI / XALAN code
> coming from our dependencies require classes outside of the JRE (e.g.
> the javax.servlet specification, org.apache.bcel).
> 
> By default the sniffer plugin doesn’t scan dependencies, but due to
> the way I’ve inlined the classes they’re now considered part of the
> project.
> 
> 
> > What does that mean in practical terms? Do we require Java 11? I'm
> > a
> > bit confused since that method is not part of the JRE.
> 
> No, we don’t. I can fix the issue for the Java 8 builds by just
> inlining the classes I mentioned in the prepare-package phase.

Sorry for the late reply. I'm not sure I follow. If you inline classes
that require Java 11, doesn't the bundle effectively require Java 11?

Robert



[CANCELLED][VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-18 Thread Radu Cotescu
Let’s cancel it and start fresh.

> On 15 Nov 2019, at 16:36, Radu Cotescu  wrote:
> 
> 
> Hi,
> 
> Builds on Java 8 fail [0] due to how I handled dependency inlining for 
> SLING-8847 [1], since the sniffer plugin will also check the dependencies’ 
> classes. Those classes should be inlined in the prepare-package phase instead 
> of generate-resources.
> I was building locally with Java 11, hence why I didn’t see any issues before 
> starting the release.
> 
> Do you want to cancel the release or can this be fixed afterwards?
> 
> Thanks,
> Radu
> 
> [0] - 
> https://builds.apache.org/job/Sling/job/sling-org-apache-sling-xss/job/master/66/display/redirect
>  
> 
> [1] - https://issues.apache.org/jira/browse/SLING-8847 
> 


Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-15 Thread Radu Cotescu
Hi Robert

> On 15 Nov 2019, at 16:40, Robert Munteanu  wrote:
> 
> Hi Radu,
> 
> On Fri, 2019-11-15 at 16:36 +0100, Radu Cotescu wrote:
>> Hi,
>> 
>> Builds on Java 8 fail [0] due to how I handled dependency inlining
>> for SLING-8847 [1], since the sniffer plugin will also check the
>> dependencies’ classes. Those classes should be inlined in the
>> prepare-package phase instead of generate-resources.
>> I was building locally with Java 11, hence why I didn’t see any
>> issues before starting the release.
> 
> IIUC, the error is
> 
> [ERROR] /home/jenkins/jenkins-slave/workspace/ling-org-apache-sling-
> xss_master/target/classes/org/owasp/esapi/tags/BaseEncodeTag.class:29:
> Undefined reference: void
> javax.servlet.jsp.tagext.BodyTagSupport.()

Because I’ve inlined the dependencies’ classes in the generate-resources phase, 
the animal-sniffer plugin will consider those classes as part of the project. 
Apparently some ESAPI / XALAN code coming from our dependencies require classes 
outside of the JRE (e.g. the javax.servlet specification, org.apache.bcel).

By default the sniffer plugin doesn’t scan dependencies, but due to the way 
I’ve inlined the classes they’re now considered part of the project.


> 
> What does that mean in practical terms? Do we require Java 11? I'm a
> bit confused since that method is not part of the JRE.


No, we don’t. I can fix the issue for the Java 8 builds by just inlining the 
classes I mentioned in the prepare-package phase.

Regards,
Radu

Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-15 Thread Robert Munteanu
Hi Radu,

On Fri, 2019-11-15 at 16:36 +0100, Radu Cotescu wrote:
> Hi,
> 
> Builds on Java 8 fail [0] due to how I handled dependency inlining
> for SLING-8847 [1], since the sniffer plugin will also check the
> dependencies’ classes. Those classes should be inlined in the
> prepare-package phase instead of generate-resources.
> I was building locally with Java 11, hence why I didn’t see any
> issues before starting the release.

IIUC, the error is

[ERROR] /home/jenkins/jenkins-slave/workspace/ling-org-apache-sling-
xss_master/target/classes/org/owasp/esapi/tags/BaseEncodeTag.class:29:
Undefined reference: void
javax.servlet.jsp.tagext.BodyTagSupport.()

What does that mean in practical terms? Do we require Java 11? I'm a
bit confused since that method is not part of the JRE.

Robert



Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-15 Thread Radu Cotescu

Hi,

Builds on Java 8 fail [0] due to how I handled dependency inlining for 
SLING-8847 [1], since the sniffer plugin will also check the dependencies’ 
classes. Those classes should be inlined in the prepare-package phase instead 
of generate-resources.
I was building locally with Java 11, hence why I didn’t see any issues before 
starting the release.

Do you want to cancel the release or can this be fixed afterwards?

Thanks,
Radu

[0] - 
https://builds.apache.org/job/Sling/job/sling-org-apache-sling-xss/job/master/66/display/redirect
 

[1] - https://issues.apache.org/jira/browse/SLING-8847 


Re: [VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-15 Thread Radu Cotescu
+1

> On 15 Nov 2019, at 15:53, Radu Cotescu  wrote:
> 
> Please vote to approve this release:
> 
>  [ ] +1 Approve the release
>  [ ]  0 Don't care
>  [ ] -1 Don't release, because ...



[VOTE] Release Apache Sling XSS Protection API 2.1.12

2019-11-15 Thread Radu Cotescu
Hi,

We solved 2 issues in this release:
https://issues.apache.org/jira/browse/SLING/fixforversion/12346517

Staging repository:
https://repository.apache.org/content/repositories/orgapachesling-2155/

You can use this UNIX script to download the release and verify the signatures:
https://gitbox.apache.org/repos/asf?p=sling-tooling-release.git;a=blob;f=check_staged_release.sh;hb=HEAD

Usage:
sh check_staged_release.sh 2155 /tmp/sling-staging

Please vote to approve this release:

  [ ] +1 Approve the release
  [ ]  0 Don't care
  [ ] -1 Don't release, because ...

This majority vote is open for at least 72 hours.

Regards,
Radu Cotescu