Bug #4732
closed
decodeControlParameters should not throw on unrecognized TLVs
0%
Description
The current behavior breaks all consumers every time the NFD management protocol adds a new field to the ControlParameters structure.
I think this regressed in commit c06427b310a07f3a96d5592bf136a070e5fea11d, that made the existing finishNestedTlvs default to skipCritical=false. That is indeed the correct v0.3 behavior for network-layer TLV elements, but the NFD management protocol is an application-layer protocol and the evolvability rules don't apply to it.
Please revert to the previous behavior of silently ignoring unknown TLVs nested in ControlParameters and ControlResponse.
Updated by Davide Pesavento over 5 years ago
Calling finishNestedTlvs(..., true) from decodeControlParameters() may be enough to fix, but I haven't tested.
Also, the API documentation of finishNestedTlvs() doesn't mention unrecognized critical elements as a reason for throwing.
Updated by Anonymous over 5 years ago
Hi Davide. I pushed a changed to master. Can you give it a try? (Are you able to build from source?)
https://github.com/named-data/jndn/commit/fb547eacad3d11b1ceaa70f653bd0d910e211b2c
Using finishNestedTlvs(..., true) will only skip unrecognized elements and the end of the TLV after recognized ones. If there are unrecognized TLVs the come before a recognized optional element which is present, then it might not be picked up. (The TlvDecoder does not handle TLVs in random order.)
Updated by Anonymous over 5 years ago
- Status changed from New to Feedback
I updated decodeControlParameters to also skip intermediate optional unused TLVs such as LocalUri.
https://github.com/named-data/jndn/commit/dba986b3ddd8d86b88c9b2f43ca9dab9703fe4a8
I pushed a new minor release to Maven Central.
https://github.com/named-data/jndn/releases/tag/v0.20
Let me know if it fixes the problem.
Updated by Davide Pesavento over 5 years ago
Jeff Thompson wrote:
Can we close this?
I haven't been able to test the new version yet, but the updated code looks fine now. Feel free to close.