Bug #3803
open
Default-constructed Link is encoded with ContentType==BLOB if no delegation is added
Added by Muhammad Hosain Abdullahi Sabet over 7 years ago. Updated almost 2 years ago.
0%
Description
Snippet to reproduce:
// g++ -std=c++14 -o x x.cpp $(pkg-config --cflags --libs libndn-cxx)
#include <ndn-cxx/link.hpp>
#include <ndn-cxx/security/key-chain.hpp>
#include <iostream>
int main()
{
ndn::Link link;
link.setName("/link-name");
ndn::KeyChain keyChain;
keyChain.sign(link);
std::cout << link;
}
Expected: ContentType==LINK, or an exception is thrown
Actual: ContentType==BLOB
Updated by Junxiao Shi over 7 years ago
- Description updated (diff)
I can't fully understand the problem. Can you post a code snippet the reproduce the problem? If it isn't posted within next 7 days, this issue will be rejected.
Updated by Muhammad Hosain Abdullahi Sabet over 7 years ago
This is a sample ndnSIM code snippet:
ndn::Name linkName = m_prefix;
auto linkObject = make_shared < ::ndn::Link>(linkName.append(ndn::Name("/Link")));
ndn::Signature sign;
ndn::SignatureInfo signInfo(static_cast< ::ndn::tlv::SignatureTypeValue>(255));
sign.setInfo(signInfo);
//linkObject->addDelegation(1,Name("/google"));
//linkObject->addDelegation(2,Name("/Verizon"));
sign.setValue(::ndn::nonNegativeIntegerBlock(::ndn::tlv::SignatureValue, 0));
linkObject->setSignature(sign);
m_appLink->onReceiveData(*linkObject);
NS_LOG_DEBUG("This data has been sent"<<*linkObject);
Having above, below will be printed:
This data has been sentName: /LinkHolder/Link
MetaInfo: ContentType: 0
Content: (size: 0)
Signature: (type: 255, value_length: 1)
Updated by Junxiao Shi over 7 years ago
- Subject changed from LINKs without DelegationName considered as ContentType_Blob to ndn::Link is encoded as ContentType==BLOB if no delegation is added
- Description updated (diff)
- Category set to Base
- Target version set to v0.6
While I can confirm the problem with a snippet that does not involve ndnSIM, I'm unsure whether this is indeed a bug, because the semantics of a Link without delegation is unclear.
We may as well consider a Link without delegation as invalid. But in that case, Link::wireEncode should throw an exception instead of encoding as BLOB.
Updated by Muhammad Hosain Abdullahi Sabet over 7 years ago
Junxiao Shi wrote:
While I can confirm the problem with a snippet that does not involve ndnSIM, I'm unsure whether this is indeed a bug, because the semantics of a Link without delegation is unclear.
I've been thinking on whether or not this is a bug, from the beginning. I think this is a bug in a sense that having this packet with ContentType=0, currently nothing indicates what the problem is, which is a Link object not having delegation name. So nether consumer nor producer can find the problem unless, like in my case, one already knows.
Updated by Muhammad Hosain Abdullahi Sabet over 7 years ago
Junxiao Shi wrote:
We may as well consider a Link without delegation as invalid. But in that case,
Link::wireEncodeshould throw an exception instead of encoding as BLOB.
Currently there is no Link::wireEncode. We use Data::wireEncode and Link::encodeContent for encoding. I can add the function to ::ndn::Link temporarily, but isn't it better to clarify the semantics of a Link object without delegationName?
Since new version of ndnSIM is not released yet, could we add this functioanilty into the associated version of ndn-cxx?
Updated by Junxiao Shi over 7 years ago
- Description updated (diff)
- Target version deleted (
v0.6)
20161020 call decides that:
- Link without delegation is invalid, according to #2587 link.pdf definition.
- The proper behavior is to throw an exception during
Link::wireEncode, rather than encoding into an invalid packet. - Since this situation is purely hypothetical, there's no urgent need to fix this bug.
- Alex is willing to accept the proposed patch as long as it does not require major refactoring.
Updated by Muhammad Hosain Abdullahi Sabet over 7 years ago
- Status changed from New to In Progress
Updated by Junxiao Shi almost 7 years ago
- Description updated (diff)
- Status changed from In Progress to New
Updated by Davide Pesavento almost 2 years ago
- Description updated (diff)
- Start date deleted (
10/10/2016)
Updated by Davide Pesavento almost 2 years ago
- Status changed from New to Closed
I can no longer reproduce with the latest ndn-cxx. The snippet in the description correctly prints:
Name: /link-name
MetaInfo: [ContentType: 1]
Content: [0 bytes]
Signature: [type: SignatureSha256WithEcdsa, length: 72]
Updated by Davide Pesavento almost 2 years ago
- Subject changed from ndn::Link is encoded as ContentType==BLOB if no delegation is added to Default-constructed Link is encoded with ContentType==BLOB if no delegation is added
- Status changed from Closed to New
But I can reproduce with a slightly modified snippet...
int main()
{
ndn::Link link;
link.setName("/link-name");
ndn::KeyChain keyChain;
keyChain.sign(link);
std::cout << link;
}
which prints:
Name: /link-name
MetaInfo: [ContentType: 0]
Signature: [type: SignatureSha256WithEcdsa, length: 71]
Updated by Davide Pesavento almost 2 years ago
Junxiao Shi wrote in #note-6:
- The proper behavior is to throw an exception during
Link::wireEncode, rather than encoding into an invalid packet.
This is infeasible in the current design because wireEncode is not virtual. As I said several times in other contexts, I believe deriving from Data was a mistake. Data is not designed to be subclassed.
Updated by Junxiao Shi almost 2 years ago
Davide Pesavento wrote in #note-13:
deriving from
Datawas a mistake.Datais not designed to be subclassed.
I ran into similar problem when designing Certificate class in NDNts.
While it's easy to have virtual function in JavaScript (every function is in fact virtual), Certificate inheriting from Data has the drawback of constructing Data twice, and the caller can accidentally invalidate the Certificate structurally by assigning to Data fields.
The solution in NDNts is making data a field of Certificate.
In C++ syntax it looks like:
class Certificate
{
public:
static Certificate fromData(Data data);
Data getData() const;
Name getName() const;
Name getKeyName() const;
ValidityPeriod getValidityPeriod() const;
span<uint8_t> getPublicKeySpki() const;
private:
Certificate(...);
};
Certificate class is immutable so that caller cannot invalidate it structurally by assignment.
Functions that create a Certificate, such as KeyChain::makeCertificate, constructs a Data first then calls Certificate::fromData.
There's no cert.isValid(now) function because the meaning of isValid() is ambiguous.
Instead, the call is cert.getValidityPeriod().includes(now).