Bug #3665
closed
Exclude doesn't support ranged operations using ImplicitSha256DigestComponent
100%
Description
The current code makes an assumption that an "empty" name component is a generic name component with empty content, which breaks the logic given ImplicitSha256DigestComponent type is smaller than GenericNameComponent.
The temporary solution would be to replace name::Component() with name::Component(Block(ImplicitSha256DigestComponent)).
The correct solution would be relax requirements (and update TLV spec) on what types name components can carry and then use name::Component(Block(0)).
Snippet to reproduce from Jeff Thompson:
BOOST_AUTO_TEST_CASE(ExcludeGenericImplicitDigest)
{
uint8_t WIRE[32] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
Exclude exclude;
exclude.excludeBefore(name::Component("C"));
name::Component component = name::Component::fromImplicitSha256Digest(WIRE, sizeof(WIRE));
BOOST_CHECK_EQUAL(exclude.isExcluded(component), true);
}
Updated by
Updated by Junxiao Shi almost 10 years ago
- Tracker changed from Task to Bug
- Description updated (diff)
- Assignee set to Junxiao Shi
- Target version set to v0.5
- Estimated time set to 3.00 h
I'll give this a try.
Updated by Anonymous almost 10 years ago
A helpful hint: When you fix this, you may find that the following unit test fails. (It's how I found the bug.)
https://github.com/named-data/ndn-cxx/blob/e4f8c3bec24566cfe7bfcc505ab39b0bf124ba98/tests/unit-tests/interest.t.cpp#L1044
This is because it matches the implicit digest against the Exclude created on line 1030.
https://github.com/named-data/ndn-cxx/blob/e4f8c3bec24566cfe7bfcc505ab39b0bf124ba98/tests/unit-tests/interest.t.cpp#L1030
Updated by Junxiao Shi almost 10 years ago
- Status changed from New to In Progress
- % Done changed from 0 to 40
https://gerrit.named-data.net/2925 patchset1 has the test cases. Implementation comes later.
Updated by Junxiao Shi almost 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 40 to 100
https://gerrit.named-data.net/2925 patchset3 completes the fix.
Updated by Junxiao Shi almost 10 years ago
- Status changed from Code review to Closed
Updated by Junxiao Shi almost 10 years ago
- Status changed from Closed to Feedback
ndncatchunks is broken by commit:df4b24e3d57f715474d17a16136e340571f92289.
https://travis-ci.org/yoursunny/ndn-cxx-breaks/jobs/145665667#L1026-L1041
error: no matching function for call to ‘ndn::Exclude::excludeOne(const ndn::Exclude::ExcludeComponent&)’
note: ndn::Exclude& ndn::Exclude::excludeOne(const ndn::name::Component&)
note: no known conversion for argument 1 from ‘const ndn::Exclude::ExcludeComponent’ to ‘const ndn::name::Component&’
Calling code looks like:
Exclude exclude1;
Exclude exclude2;
for (const auto& i : exclude1) {
exclude2.excludeOne(i.first);
}
This code uses low-level ExcludeEntry API to enumerate the entries, and calls high-level excludeOne API to insert entries into another Exclude instance.
The quick solution is to allow implicit conversion from Exclude::ExcludeComponent to name::Component, or to accept ExcludeComponent in high-level APIs.
However, this is semantically wrong, because it's unclear what "negative infinity ExcludeComponent" means in high-level API.
The intention of calling code is to merge exclude1 into exclude2.
I think Exclude type could overload operator+= to explicitly support this use case.
Updated by Junxiao Shi almost 10 years ago
20160719 call discussed note-7 problem.
The use case of Exclude low-level API is to allow a caller to inspect what is in the Exclude.
We need to redesign Exclude enumeration API to expose "Range" as units, and then fix ndncatchunks accordingly.
Updated by Junxiao Shi almost 10 years ago
- Related to Feature #3681: Exclude range enumeration added
Updated by Junxiao Shi almost 10 years ago
- Status changed from Feedback to Closed
note-8 work is split to #3681.
Updated by Junxiao Shi almost 10 years ago
ndncatchunksis broken by commit:df4b24e3d57f715474d17a16136e340571f92289.
https://travis-ci.org/yoursunny/ndn-cxx-breaks/jobs/145665667#L1026-L1041
This is fixed in https://gerrit.named-data.net/2959.
Updated by Junxiao Shi almost 10 years ago
- Related to Bug #3693: integ fails due to improper Exclude usage added