Project

General

Profile

Actions

Bug #4489

closed

ConfigFile::parseNumber<unsigned>() doesn't reject negative arguments

Added by Davide Pesavento almost 7 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Core
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

For instance, ConfigFile::parseNumber<uint16_t> is used in the TCP and UDP factories to parse the port number. Negative port numbers are accepted instead of raising an exception.

The following test case should pass but currently fails.

BOOST_AUTO_TEST_CASE(BadPort)
{
  const std::string CONFIG = R"CONFIG(
    face_system
    {
      udp
      {
        port -1
      }
    }
  )CONFIG";

  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
}

Related issues 1 (1 open0 closed)

Related to NFD - Task #5091: Switch NFD configuration file formatNew

Actions
Actions #1

Updated by Junxiao Shi over 6 years ago

name::Component::fromEscapedString solved a similar problem with the following code:

long parsedType = std::strtol(input.data(), nullptr, 10);
if (parsedType < tlv::NameComponentMin || parsedType > tlv::NameComponentMax ||
    to_string(parsedType).size() != equalPos) {
  BOOST_THROW_EXCEPTION(Error("Incorrect TLV-TYPE in NameComponent URI"));
}

It converts the parsed number back to a string, and checks that this string equals the input string.

Adopting the same logic to ConfigFile::parseNumber implies that numbers written in non-canonical form, such as 06363 or +6363, would be rejected. I think it's okay to require numbers to be in canonical form.

Actions #2

Updated by Davide Pesavento over 6 years ago

Junxiao Shi wrote:

Adopting the same logic to ConfigFile::parseNumber implies that numbers written in non-canonical form, such as 06363 or +6363, would be rejected.

06363 would be interpreted as octal, wouldn't it?

I think it's okay to require numbers to be in canonical form.

I think so too.

Actions #3

Updated by Davide Pesavento over 5 years ago

This seems to have been fixed somewhat recently in boost, probably in version 1.68.0 (Aug 2018), although I couldn't isolate the exact commit (boost::property_tree hasn't been touched since Oct 2016).

Actions #4

Updated by Eric Newberry over 4 years ago

Davide Pesavento wrote:

This seems to have been fixed somewhat recently in boost, probably in version 1.68.0 (Aug 2018), although I couldn't isolate the exact commit (boost::property_tree hasn't been touched since Oct 2016).

Strangely, this appears to occur on a Jenkins 10.15 agent with boost 1.72.0: https://jenkins.named-data.net/job/NFD/7561/OS=OSX-10.15/consoleFull

However, it does not occur on a 10.14 agent with the same version of boost: https://jenkins.named-data.net/job/NFD/7561/OS=OSX-10.14/consoleFull

Actions #5

Updated by Eric Newberry over 4 years ago

  • Status changed from New to In Progress
  • Assignee set to Eric Newberry
Actions #6

Updated by Eric Newberry over 4 years ago

Eric Newberry wrote:

Strangely, this appears to occur on a Jenkins 10.15 agent with boost 1.72.0: https://jenkins.named-data.net/job/NFD/7561/OS=OSX-10.15/consoleFull

However, it does not occur on a 10.14 agent with the same version of boost: https://jenkins.named-data.net/job/NFD/7561/OS=OSX-10.14/consoleFull

I just tried with Ubuntu 19.10 and Boost 1.72.0 and these test cases also failed.

Actions #7

Updated by Davide Pesavento over 4 years ago

Ok. Boost probably delegates the actual parsing to some lower-level C/C++ library function, which may have accidentally changed behavior in 10.14, and reverted back to the previous behavior in 10.15. Just a guess...

Actions #8

Updated by Eric Newberry over 4 years ago

  • Related to Task #5091: Switch NFD configuration file format added
Actions #9

Updated by Eric Newberry over 4 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100
Actions #10

Updated by Eric Newberry over 4 years ago

  • Status changed from Code review to Closed
Actions #11

Updated by Davide Pesavento over 4 years ago

  • Target version set to 22.02
Actions

Also available in: Atom PDF