RFR: JDK-6515161 If remote removeNotificationListener gets SecurityException, client no longer gets notifications

[Daniel Fuchs]

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,
>>>>
>>>
>>
>