Fix getRegistrationByAdress (use index and handle address conflict) #517
Conversation
|
Based on the question, what I see is that we are discarding registrations having the same ip address, shouldn't this cause problems with clients behind the same NAT? |
|
What do you mean "we are discarding registrations having the same ip address" ? Registration was not discarded but the index (addr => registration) keeps the most recent registration. |
|
From what I see this doesn't not resolve the actual question, in my opinion, the ideal scenario would be to return a list of registrations instead of a single one. |
|
Ok, let me resume that, in a leshan server context : PskStore.getIdentity(InetSocketAddress inetAddress) is used when the server want to initiate a DTLS connection. This should return only 1 identity, this make sense as we want to talk to 1 specific device/client. In Leshan the way to know the psk identity, is to search in RegistrationStore to find which registration is associated to this socket address. Theoretical there is only 1 device for 1 address, but with NAT a device D1 could get an address A then go to sleep, then a new device D2 will get this free address A. So in the store 2 devices with the same address A. But in fact there is only 1 device behind the A address :D2. D1 lost this address and we don't know its new address, we will know that next time it will update its registration. So this makes senses to me to only keep tracks to the device which use the address A lastly.
I'm not sure to understand what is the question ? :)
I do not understand what does it bring more, as pskStore must choose one of them to choose 1 identity. |
|
Sounds reasonable, thanks! |
| byte[] regid_idx = toRegIdKey(registration.getId()); | ||
| j.set(regid_idx, registration.getEndpoint().getBytes(UTF_8)); | ||
| byte[] addr_idx = toRegAddrKey(registration.getSocketAddress()); | ||
| j.set(addr_idx, registration.getEndpoint().getBytes(UTF_8)); | ||
|
|
||
| if (old != null) { | ||
| Registration oldRegistration = deserializeReg(old); | ||
| // remove old secondary index | ||
| if (registration.getId() != oldRegistration.getId()) |
There was a problem hiding this comment.
Not related to this PR but this string equality looks buggy...
There was a problem hiding this comment.
I will fix this in a separated commit
See #464 for more details.