tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Williams <nicho...@nicholaswilliams.net>
Subject Re: svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Wed, 11 Sep 2013 16:33:27 GMT

On Sep 11, 2013, at 9:22 AM, Konstantin Kolinko wrote:

> 2013/9/11 Mark Thomas <markt@apache.org>:
>> On 11/09/2013 14:44, Konstantin Kolinko wrote:
>>> 2013/9/11  <markt@apache.org>:
>>>> Author: markt
>>>> Date: Wed Sep 11 11:59:37 2013
>>>> New Revision: 1521817
>>>> 
>>>> URL: http://svn.apache.org/r1521817
>>>> Log:
>>>> Comment
>>>> 
>>>> Modified:
>>>>    tomcat/tc6.0.x/trunk/STATUS.txt
>>>> 
>>>> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
>>>> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1521817&r1=1521816&r2=1521817&view=diff
>>>> ==============================================================================
>>>> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
>>>> +++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Sep 11 11:59:37 2013
>>>> @@ -103,6 +103,10 @@ PATCHES PROPOSED TO BACKPORT:
>>>>      I think @Target change for @DenyAll is wrong.
>>>>      Looking at Geronimo, @DenyAll has "@Target({ElementType.METHOD})" in
CA 1.0 there.
>>>>      It is "@Target({ElementType.TYPE, ElementType.METHOD})" starting with
CA 1.1.
>>>> +     markt:
>>>> +     The CA 1.0 spec section 2.11 is explicit that DenyAll is permitted
on
>>>> +     classes. Geronimo and whatever source was used generate the official
Java
>>>> +     EE 5 Javadoc are wrong.
>>> 
>>> Ah, I see it.
>>> 
>>> Nevertheless, it looks to me that it is not just a typo, but a genuine
>>> error, that was corrected in CA 1.1. It is mentioned in changelog,
>>> http://jcp.org/aboutJava/communityprocess/maintenance/jsr250/250ChangeLog.html
>>> -> "Maintenance Review 1," -> "2. Change the definition of the
>>> @DenyAll annotation"
>> 
>> That looks like a Javadoc / implementation correction to me. The EG's
>> aren't always very good at keeping spec issues and RI issues separate.
>> 
>>> Unless there is some evidence in mailing lists elsewhere, I think the
>>> question is which version of the class would pass a TCK. I think that
>>> Geronimo classes might have been tested better, than ones in Tomcat.
>> 
>> If the Tomcat version failed a TCK, I'd challenge the failure on the
>> grounds of the CA 1.0 spec section 2.11.
>> 
> 
> I would like to see either someone encountering and reporting this
> issue in Tomcat 6,
> or some existing implementation of CA 1.0 that has this change.
> 
> I do not see enough grounds to change this class in Tomcat 6 now, It is legacy.
> 
> 
> Just googling, trying to find whether others noted this issue.
> 
> http://www.oracle.com/technetwork/articles/javaee/security-annotation-142276.html
> does not have 'X' in "@DenyAll vs. TYPE" cell in a table.
> 
> http://pic.dhe.ibm.com/infocenter/rsawshlp/v7r5m0/index.jsp?topic=%2Fcom.ibm.jee5.doc%2Ftopics%2Ftsecuringejee.html
> does not say that @DenyAll can be used at type level

Some more info that might be helpful:

1) Fixed in GlassFish v3 on April 29, 2009: https://java.net/jira/browse/GLASSFISH-8045
2) Checkout out GlassFish v2 source and DenyAll is METHOD only. Not TYPE. It was never fixed
in v2.

Now, with that said, I agree with Mark. If a TCK failed because of DenyAll having TYPE, I
would challenge it. The spec clearly says it should have TYPE. This change would make Tomcat
6 the only compliant implementation.

The question is, are there any *downsides* to being strictly spec compliant here? I don't
see any. So the annotation is available for TYPE when other implementations don't allow it?
That's not going to break any application code trying to run on Tomcat.

It's *correct.* I say we go with it.
Mime
View raw message