RFR: JDK-6515161 If remote removeNotificationListener gets SecurityException, client no longer gets notifications
Harsha Wardhana B
harsha.wardhana.b at oracle.com
Mon May 8 16:13:41 UTC 2017
Hi Daniel,
The term 'listenerid' is used in conjunction with method name/object
field which adds context about the term 'listenerid'. Having a
standalone method name as getClientListenerId is less ambiguous compared
to method name : getListenerId.
I don't really have a strong opinion on this and am okay with either names.
-Harsha
On Monday 08 May 2017 03:16 PM, Daniel Fuchs wrote:
> On 08/05/2017 09:45, Ujwal Vangapally wrote:
>> Thanks for the Review Daniel, Harsha
>>
>> Please find the new webrev incorporating the review comments
>>
>> webrev :
>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.01/
>
> Looks good. In retrospect I wonder if renaming
> getListenerIds/getListenerId into
> getClientListenerIds/getClientListenerId
> was such a good idea given that 'listenerId' is an established
> terminology in the specification [1] [2] (and 'listenerId' is used all
> over the place in ClientNotifForwarder anyway).
>
> Harsha, what do you think?
>
> best regards,
>
> -- daniel
>
> [1] See @return in
> http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#addNotificationListeners-javax.management.ObjectName:A-java.rmi.MarshalledObject:A-javax.security.auth.Subject:A-
> [2] see @param listenerIds in
> http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#removeNotificationListeners-javax.management.ObjectName-java.lang.Integer:A-javax.security.auth.Subject-
>
>
>
>>
>> Thanks,
>>
>> Ujwal
>>
>>
>> On 5/8/2017 12:03 PM, Daniel Fuchs wrote:
>>> Hi Ujwal,
>>>
>>> For consistency I think the new getListenerIds method should:
>>>
>>> a) either return an array of Integer, even if it contains only 1
>>> Integer:
>>>
>>> 1. The name of the method implies that an array is returned
>>> 2. You will need the array when you call
>>> connection.removeNotificationListeners anyway.
>>>
>>> or b) the other possibility is to remove the 's' at the end of the
>>> method.
>>>
>>> Both would be acceptable.
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 04/05/17 07:59, Ujwal Vangapally wrote:
>>>> corrected webrev link :
>>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/
>>>>
>>>>
>>>> On 5/4/2017 12:14 PM, Ujwal Vangapally wrote:
>>>>>
>>>>> Kindly review the changes made for below bug
>>>>>
>>>>> Problem description and solution are explained in comments section
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-6515161
>>>>>
>>>>> diff for*ClientNotifForwarder.java *might be a bit confusing as it
>>>>> shows the method name
>>>>>
>>>>> removeNotificationListener is modified to getListenerIds and new
>>>>> method removeNotificationListener is introduced.
>>>>>
>>>>> Actual change is new method getListenerIds is introduced and it is
>>>>> called in removeNotificationListener method without
>>>>>
>>>>> affecting the functionality of removeNotificationListener.
>>>>>
>>>>> webrev :
>>>>> cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Ujwal,
>>>>>
>>>>
>>>
>>
>
More information about the serviceability-dev
mailing list