NordicSemiconductor / Android-nRF-Mesh-Library

The Bluetooth Mesh Provisioner and Configurator library.
https://www.nordicsemi.com/
BSD 3-Clause "New" or "Revised" License
414 stars 177 forks source link

Crash app in sendBlockAck #462

Closed ghost closed 2 years ago

ghost commented 2 years ago

Describe the bug When the library receives a segmented message that needs a block ack from an unknown node, the app will crash due to a NullPointerException.

To Reproduce Steps to reproduce the behavior:

  1. provision two nodes to the network
  2. manually delete one node from the database without sending a ConfigNodeReset
  3. connect to the second node of this network
  4. make the first node to send a message that makes the library to trigger block ack answer

Expected behavior Probably log some warning that the node is unknown instead of trying to send a block ack ?

Platform details:

Logs / Screenshots image

roshanrajaratnam commented 2 years ago

Do you use the proxy filter in this case to receive the message on the phone that is send to other nodes?

ghost commented 2 years ago

No, we still have to integrate this I think...at least I never worked on this feature, and I know we should (embedded team request)

ghost commented 2 years ago

Sooo, sorry but the answer is maybe ? 😅 consider we don't

roshanrajaratnam commented 2 years ago

so you basically delete the node, connect to and send some message that's segmented? I'll test this.

ghost commented 2 years ago

connect to and

connect to another one, yes

ghost commented 2 years ago

it's not the phone that initiate the msg actually, it's the unkown node

roshanrajaratnam commented 2 years ago

Could you detail how to set it up please? it get's a bit confusing.

ghost commented 2 years ago

hmmm 🤔

Well you can also simulate it with two phones I think :

roshanrajaratnam commented 2 years ago

ok that should help me to produce this. What's the segmented message you are sending here?

ghost commented 2 years ago

😅 I think it was proprietary message. So I never tested with other ones

roshanrajaratnam commented 2 years ago

By just looking at the code I don't see how this could happen because the message is not processed any further if the node does not exist. https://github.com/NordicSemiconductor/Android-nRF-Mesh-Library/blob/e03b1eba6cf4d7bb41b1fa36d3ce8055e8df4208/mesh/src/main/java/no/nordicsemi/android/mesh/transport/BaseMeshMessageHandler.java#L127

ghost commented 2 years ago

Well, yeh I would expect the library to do such check indeed... The thing is, it happens ahah. To be more precise on our case, it was happening because we let an external BLE device to be a provisioner without declaring it on the phone (prototyping where phone was not compatible with this new device). This external device was sending a proprietary segmented message (extending ApplicationMessage) to a known node in order to read some config. As soon as we declared the provisioner and pass it to the external device, the library was not failing anymore.

ghost commented 2 years ago

it can also happen if a provisioner change unicast without the phone knowing about it (shared network)

ghost commented 2 years ago

proprietary message

If it can help you, here is the implem for our fork of this library

roshanrajaratnam commented 2 years ago

@R0m4in-dooz does the fix made in #461 fixed this one?

ghost commented 2 years ago

Hello @roshanrajaratnam ! Unfortunately I'm not able to test it as of now..but as I explained earlier in this ticket, it's pretty easy to reproduce as long as you have two phones and two nodes. I may have time to test next week tho 👍

roshanrajaratnam commented 2 years ago

I gave this a test now and I am not able to reproduce this, I used the mesh chat example from the nRF Connect SDK where you can send a long message to a specific node. The fix from #461 have solved the issue in my humble opinion.