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/06/27 13:08:03 UTC

svn commit: r1497299 - /tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java

Author: markt
Date: Thu Jun 27 11:08:03 2013
New Revision: 1497299

URL: http://svn.apache.org/r1497299
Log:
WebSocket 1.0, Section 4.8
Don't look for annotations on inherited methods.

Modified:
    tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java

Modified: tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java?rev=1497299&r1=1497298&r2=1497299&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java Thu Jun 27 11:08:03 2013
@@ -77,7 +77,7 @@ public class PojoMethodMapping {
         Method open = null;
         Method close = null;
         Method error = null;
-        for (Method method : clazzPojo.getMethods()) {
+        for (Method method : clazzPojo.getDeclaredMethods()) {
             if (method.getAnnotation(OnOpen.class) != null) {
                 if (open == null) {
                     open = method;



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


Re: svn commit: r1497299 - /tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Jun 27, 2013, at 9:42 AM, Mark Thomas wrote:

> On 27/06/2013 15:29, Nick Williams wrote:
>> 
>> On Jun 27, 2013, at 6:08 AM, markt@apache.org wrote:
>> 
>>> Author: markt
>>> Date: Thu Jun 27 11:08:03 2013
>>> New Revision: 1497299
>>> 
>>> URL: http://svn.apache.org/r1497299
>>> Log:
>>> WebSocket 1.0, Section 4.8
>>> Don't look for annotations on inherited methods.
>>> 
>>> Modified:
>>>   tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
>>> 
>>> Modified: tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
>>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java?rev=1497299&r1=1497298&r2=1497299&view=diff
>>> ==============================================================================
>>> --- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java (original)
>>> +++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java Thu Jun 27 11:08:03 2013
>>> @@ -77,7 +77,7 @@ public class PojoMethodMapping {
>>>        Method open = null;
>>>        Method close = null;
>>>        Method error = null;
>>> -        for (Method method : clazzPojo.getMethods()) {
>>> +        for (Method method : clazzPojo.getDeclaredMethods()) {
>>>            if (method.getAnnotation(OnOpen.class) != null) {
>> 
>> This will return non-public methods as well as public methods. Are non-public WebSocket-annotated methods permitted?
> 
> The spec is silent on that particular issue.

So maybe some context would help. Are there other cases elsewhere in the JavaEE spec where annotations mark methods as handling some action? If so, are those methods allowed to be non-public? I would suggest, in absence of the spec saying exactly, that being consistent with similar cases seems right. Of course, clarification from the EG always helps...

> 
>> If not, would it be better to throw an exception here if(!Modifier.isPublic(method.getModifiers())) instead of allowing configuration to succeed and then receiving an IllegalAccessException during a session, possibly after it has been active for some time?
> 
> The other option is to ignore them (which is what happened before).
> 
> I'm leaning towards the Exception approach.

Agreed. If there are non-public annotated methods the endpoint is invalid. A connection should never be permitted to an invalid endpoint. The best way to avoid that is to throw an exception here.

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


Re: svn commit: r1497299 - /tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java

Posted by Mark Thomas <ma...@apache.org>.
On 27/06/2013 15:29, Nick Williams wrote:
> 
> On Jun 27, 2013, at 6:08 AM, markt@apache.org wrote:
> 
>> Author: markt
>> Date: Thu Jun 27 11:08:03 2013
>> New Revision: 1497299
>>
>> URL: http://svn.apache.org/r1497299
>> Log:
>> WebSocket 1.0, Section 4.8
>> Don't look for annotations on inherited methods.
>>
>> Modified:
>>    tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
>>
>> Modified: tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java?rev=1497299&r1=1497298&r2=1497299&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java (original)
>> +++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java Thu Jun 27 11:08:03 2013
>> @@ -77,7 +77,7 @@ public class PojoMethodMapping {
>>         Method open = null;
>>         Method close = null;
>>         Method error = null;
>> -        for (Method method : clazzPojo.getMethods()) {
>> +        for (Method method : clazzPojo.getDeclaredMethods()) {
>>             if (method.getAnnotation(OnOpen.class) != null) {
> 
> This will return non-public methods as well as public methods. Are non-public WebSocket-annotated methods permitted?

The spec is silent on that particular issue.

> If not, would it be better to throw an exception here if(!Modifier.isPublic(method.getModifiers())) instead of allowing configuration to succeed and then receiving an IllegalAccessException during a session, possibly after it has been active for some time?

The other option is to ignore them (which is what happened before).

I'm leaning towards the Exception approach.

Mark


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


Re: svn commit: r1497299 - /tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Jun 27, 2013, at 6:08 AM, markt@apache.org wrote:

> Author: markt
> Date: Thu Jun 27 11:08:03 2013
> New Revision: 1497299
> 
> URL: http://svn.apache.org/r1497299
> Log:
> WebSocket 1.0, Section 4.8
> Don't look for annotations on inherited methods.
> 
> Modified:
>    tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
> 
> Modified: tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java?rev=1497299&r1=1497298&r2=1497299&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java Thu Jun 27 11:08:03 2013
> @@ -77,7 +77,7 @@ public class PojoMethodMapping {
>         Method open = null;
>         Method close = null;
>         Method error = null;
> -        for (Method method : clazzPojo.getMethods()) {
> +        for (Method method : clazzPojo.getDeclaredMethods()) {
>             if (method.getAnnotation(OnOpen.class) != null) {

This will return non-public methods as well as public methods. Are non-public WebSocket-annotated methods permitted? If not, would it be better to throw an exception here if(!Modifier.isPublic(method.getModifiers())) instead of allowing configuration to succeed and then receiving an IllegalAccessException during a session, possibly after it has been active for some time?

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