You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/09/11 13:59:38 UTC

svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt

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.
   -1:
 
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Sep 11, 2013, at 9:22 AM, Konstantin Kolinko wrote:

> 2013/9/11 Mark Thomas <ma...@apache.org>:
>> On 11/09/2013 14:44, Konstantin Kolinko wrote:
>>> 2013/9/11  <ma...@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.

Re: svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/9/11 Mark Thomas <ma...@apache.org>:
> On 11/09/2013 14:44, Konstantin Kolinko wrote:
>> 2013/9/11  <ma...@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

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Mark Thomas <ma...@apache.org>.
On 11/09/2013 14:44, Konstantin Kolinko wrote:
> 2013/9/11  <ma...@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.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1521817 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/9/11  <ma...@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"

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.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org