Bug #4732

decodeControlParameters should not throw on unrecognized TLVs

Added by Davide Pesavento 10 months ago. Updated 8 months ago.

Start date:
Due date:
% Done:


Estimated time:


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.


#1 Updated by Davide Pesavento 10 months 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.

#2 Updated by Jeff Thompson 10 months ago

Hi Davide. I pushed a changed to master. Can you give it a try? (Are you able to build from source?)

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.)

#3 Updated by Jeff Thompson 10 months ago

  • Status changed from New to Feedback

I updated decodeControlParameters to also skip intermediate optional unused TLVs such as LocalUri.

I pushed a new minor release to Maven Central.

Let me know if it fixes the problem.

#4 Updated by Jeff Thompson 8 months ago

Can we close this?

#5 Updated by Davide Pesavento 8 months 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.

#6 Updated by Jeff Thompson 8 months ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF