Task #1640
closedImplicitSha256DigestComponent
100%
Description
Support ImplicitSha256DigestComponent type.
This task includes:
- develop new component class
- extend Name class to allow new component type
- extend URI encoder and decoder
- extend PIT matching algorithm
Updated by Junxiao Shi about 10 years ago
- Blocked by Task #2011: ImplicitSha256DigestComponent added
Updated by Junxiao Shi about 10 years ago
- Subject changed from NumberComponent and ImplicitSha256DigestComponent to ImplicitSha256DigestComponent
- Description updated (diff)
Updated by Alex Afanasyev about 10 years ago
- Status changed from New to In Progress
- % Done changed from 0 to 100
Updated by Alex Afanasyev about 10 years ago
- Status changed from In Progress to Code review
Updated by Junxiao Shi about 10 years ago
Change-Id: I756c4b94196cf031da98b5689bd60630533dfeb3 doesn't complete this Task.
In particular, the following is missing:
- change
Data::getFullName
to append ImplicitSha256DigestComponent instead of NameComponent - extend PIT matching algorithm, with test cases
- in Interest Name, permit ImplicitSha256DigestComponent only as the last component
- in Data Name, forbid ImplicitSha256DigestComponent
- in FinalBlockId, forbid ImplicitSha256DigestComponent
These should be added in future commit(s) also under this Task.
Updated by Junxiao Shi about 10 years ago
I have some design questions about I756c4b94196cf031da98b5689bd60630533dfeb3:
- Why is implicit digest component a state of
name::Component
class, instead of a subclass? - Given that implicit digest is always SHA256, is it necessary for "Sha256" to appear in function names, or can we drop it to make function names shorter?
Updated by Alex Afanasyev about 10 years ago
Intention of including "Sha256" is to allow (in the future) replace the hashing algorithm, when/if sha256 becomes insecure.
With this intention, "sha256" needs to be included somewhere, either in the name or in the parameters somewhere.
Updated by Junxiao Shi about 10 years ago
This question from note-9 is unanswered:
Why is implicit digest component a state of name::Component
class, instead of a subclass?
Updated by Alex Afanasyev about 10 years ago
Why not? Everything is name component, just some name components have special type.
It could have been a subclass, though I don't see much need for that. The current way (digest as a state) aligns with naming convention based elements, even though they do not have special types assigned (yet?).
Also, having it as a subclass may create some efficiency problems: something in the class must become virtual and Name parsing would need to create special instances for some name components, or there would be a need for creating an instance of digest component class before accessing the value. Given the only purpose for the special type is to provide disambiguation and there are no really any special handling (except converting to/from URI), there is no need for special subclass.
Updated by Junxiao Shi about 10 years ago
- Blocks Task #2118: ContentStore matching test cases: update for ImplictSha256DigestComponent added
Updated by Junxiao Shi about 10 years ago
The pending commit commit:6e6f8a394aa5916517b4ef2bed6e6cbcc193d52c, when rebased upon commit:1805826b47c44177b7f42484b4970b3e9c94cfe7, causes segfault in NFD test initialization.
The problem is in NFD/tests/daemon/mgmt/validation-common.hpp
.
It's equivalent to the following snippet:
#include <ndn-cxx/name.hpp>
using ndn::Name;
class A
{
public:
static const Name s_name;
};
const Name A::s_name("/A");
int
main()
{
return 0;
}
When Name
is constructed statically, the order of constructing this static Name and SHA256DIGEST_PREFIX
in ndn-cxx/src/name-component.cpp
is undetermined.
Therefore, SHA256DIGEST_PREFIX
could be unavailable when it's needed by Component::fromEscapedString
.
I've tested that changing SHA256DIGEST_PREFIX
to be a private static member of Component
class does not solve this problem.
The following solution works:
static const std::string&
getSha256DigestUriPrefix()
{
static const std::string prefix = "sha256digest=";
return prefix;
}
// and use getSha256DigestUriPrefix() instead of SHA256DIGEST_PREFIX
Updated by Junxiao Shi about 10 years ago
When Data::getFullName
is changed to append ImplicitSha256DigestComponent, ContentStore and InMemoryStorage test cases will be broken.
Please make those test cases as expected failures.
I'll fix them in #2118.
Updated by Junxiao Shi about 10 years ago
- Blocks Task #1706: Reduce implicit digest computation in ContentStore added
Updated by Alex Afanasyev about 10 years ago
- Assignee changed from Alex Afanasyev to Jiewen Tan
- % Done changed from 40 to 80
Jiewen, can you fix the test cases in http://gerrit.named-data.net/#/c/1407?
Updated by Junxiao Shi about 10 years ago
As I said in note-15, please don't fix any ContentStore and InMemoryStorage test cases in this Task. Just mark them as expected failures. I'll fix them in #2118.
Updated by Alex Afanasyev about 10 years ago
- Assignee changed from Jiewen Tan to Alex Afanasyev
- % Done changed from 80 to 100
Missed that. Will change test assertions to WARN and add /// @todo
statement
Updated by Junxiao Shi about 10 years ago
Missed that. Will change test assertions to WARN and add
/// @todo
statement
Don't change assertions to WARN. Use expected failures.
Updated by Alex Afanasyev about 10 years ago
I will add these to normal test cases, but this doesn't work with templated test cases :-(
Updated by Alex Afanasyev about 10 years ago
What did you mean by PIT matching algorithm? Interest::matchesData? Or something inside NFD?
Updated by Junxiao Shi about 10 years ago
I think Interest::matchesData
shall be sufficient because client's PIT is using linked list.
Updated by Junxiao Shi about 10 years ago
- Status changed from Code review to Closed