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 over 8 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 over 8 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 over 8 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 over 8 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 over 8 years ago
- Status changed from Code review to Closed
Updated by Junxiao Shi over 8 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 over 8 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 over 8 years ago
- Related to Feature #3681: Exclude range enumeration added
Updated by Junxiao Shi over 8 years ago
- Status changed from Feedback to Closed
note-8 work is split to #3681.
Updated by Junxiao Shi over 8 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 over 8 years ago
- Related to Bug #3693: integ fails due to improper Exclude usage added