Project

General

Profile

Actions

Bug #1528

closed

Inconsistent use of Tlv::Error as a base class

Added by Alex Afanasyev about 10 years ago. Updated almost 10 years ago.

Status:
Closed
Priority:
Normal
Category:
Base
Target version:
Start date:
04/22/2014
Due date:
% Done:

100%

Estimated time:

Description

Original report

I got an unconfirmed report that when Interest is validated, but CommandParameters are malformed, NFD does not properly handle the exception and terminates.

Most likely this problem is due to inconsistent use of exception throwing. In particular, most wire errors will trigger an exception that is derived from Tlv::Error (which is handled in various places). At the same time, some exceptions that can be caused by malformed packet do not derive from Tlv::Error. In particular, errors in Name, name::Component, Exclude, KeyLocator, Signature, and may be some others are directly inherited from runtime_error.

Actions #1

Updated by Anonymous about 10 years ago

  • Status changed from New to In Progress

Is it possible to get the code/test case that produced the exception?

Actions #2

Updated by Junxiao Shi about 10 years ago

  • Tracker changed from Task to Bug

I would say, you can reject this bug due to missing "steps to reproduce".

Actions #3

Updated by Anonymous about 10 years ago

I can confirm a problem based on visual inspection of the code. All NFD management modules rely on ManagerBase::extractParameters to catch exceptions thrown by ControlParameters::wireDecode (rib-manager uses RibManager::extractParameters, but it's the exact same code).

However, extractParameters only expects an ndn::Tlv::Error. It seems that ControlParameters::wireDecode can also throw a ControlParameters::Error:

  • "expecting TLV-TYPE ControlParameters"
  • "expecting Strategy/Name"

Obviously I can add the additional exception type to ManagerBase/RibManager::extractParameters, but I'm not sure if that's the correct fix. Should wireDecode only throw ndn::Tlv::Errors ?

Actions #4

Updated by Junxiao Shi about 10 years ago

ControlParameters::Error is a subclass of Tlv::Error

Actions #5

Updated by Alex Afanasyev about 10 years ago

What about control command? Do we handle validation of control parameters?

Actions #6

Updated by Anonymous about 10 years ago

I think so. ManagerBase/RibManager::validateParameters catches ControlCommand::AgrumentErrors thrown by validateRequest.

I just noticed Component::wireDecode throws a Component::Error which inherits directly from std::runtime_error. However, I'm not sure this code is actually being called.

Could this be causing a problem?

static_cast<LocalControlFeature>(readNonNegativeInteger(*val))

I found this: http://stackoverflow.com/questions/11551853/can-static-cast-throw-an-exception-in-c

Actions #7

Updated by Alex Afanasyev about 10 years ago

Hmm. name::Component::Error could be the problem. I'll re-assign this to myself and check exceptions in the library. I think these should be inherited from Tlv::Error as well.

Actions #8

Updated by Alex Afanasyev about 10 years ago

  • Project changed from NFD to ndn-cxx
  • Subject changed from Unhandled exception when CommandParameters are malformed to Inconsistent use of Tlv::Error as a base class
  • Description updated (diff)
  • Category deleted (Management)
  • Assignee changed from Anonymous to Alex Afanasyev
  • Target version deleted (v0.1)

I will update definition of exceptions in the library.

Actions #9

Updated by Alex Afanasyev about 10 years ago

I just noticed in the doxygen (http://named-data.net/doc/ndn-cpp-dev/0.4.0/doxygen/d8/d0b/classndn_1_1nfd_1_1ControlCommand_1_1ArgumentError.html) that ControlCommand throws another exception that is not derived from Tlv::Error/std::runtime_error.

Is ArgumentError properly handled?

Actions #10

Updated by Anonymous about 10 years ago

As far as I can tell. The only method the management modules use that throws an ArgumentError is ControlCommand::validateRequest. This call is wrapped in an appropriate try catch in ManagerBase/RibManager::validateParameters.

The management modules also use ControlCommand::applyDefaultsToRequest, but that family of methods doesn't throw any exceptions.

Actions #11

Updated by Alex Afanasyev about 10 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 80
Actions #12

Updated by Alex Afanasyev almost 10 years ago

  • Category set to Base
  • Status changed from Resolved to Closed
  • Target version set to v0.1
  • % Done changed from 80 to 100
Actions

Also available in: Atom PDF