Bug #4156
closed
Encoding LpPackets after adding repeating field causes other fields to be lost
100%
Description
Encoding an LpPacket immediately after adding a repeatable field appears to cause all other fields to be lost, as seen in this example code:
#include <ndn-cxx/lp/packet.hpp>
#include <iostream>
#include <cstring>
int
main(int argc, char** argv)
{
ndn::Buffer buf(100);
uint32_t value = 1;
std::memcpy(buf.buf(), &value, sizeof(value));
ndn::lp::Packet pkt;
pkt.set<ndn::lp::FragIndexField>(123);
std::cout << pkt.wireEncode().size() << std::endl;
pkt.set<ndn::lp::FragmentField>(std::make_pair(buf.begin(), buf.end()));
std::cout << pkt.wireEncode().size() << std::endl;
pkt.add<ndn::lp::TxSequenceField>(23568981);
std::cout << pkt.wireEncode().size() << std::endl;
pkt.add<ndn::lp::AckField>(1000);
std::cout << pkt.wireEncode().size() << std::endl;
pkt.add<ndn::lp::AckField>(1001);
std::cout << pkt.wireEncode().size() << std::endl;
pkt.add<ndn::lp::AckField>(1002);
std::cout << pkt.wireEncode().size() << std::endl;
ndn::Block b = pkt.wireEncode();
std::cout.write(reinterpret_cast<const char*>(b.wire()), b.size());
}
Producing the following output:
5
107
115
8
8
8
[plus some non-alphanumeric output of the encoded packet]
This issue appears to be fixed if the LpPacket's existing elements are parsed before finding an iterator for the new element's position.
Updated by Eric Newberry almost 7 years ago
- Blocks Feature #3931: Implement NDNLP Link Reliability Protocol added
Updated by Davide Pesavento almost 7 years ago
- Tracker changed from Task to Bug
- Category set to Base
- Target version set to v0.6
Updated by Junxiao Shi almost 7 years ago
A Block that contains sub-elements can be in one of two states:
- A parsed state, where
block.elements()vector reflects what sub-elements are contained in the block, butblock.wire()does not necessarily reflect the truth. - An encoded state, where
block.wire()is the truth, butblock.elements()may be outdated.
Block::parse assumes block.wire() is the truth, and updates block.elements() to match it. Block::encode assumes block.elements() is the truth, and updates block.wire() to match them.
This issue appears to be fixed if the LpPacket's existing elements are parsed before finding an iterator for the new element's position.
The solution seems to suggest that the block inside LpPacket should be in encoded state at all times, while each operation that accesses a sub-element must first invoke Block::parse(). Given that multiple fields in an LpPacket is accessed together, it is inefficient to do so.
How about keeping the block in parsed state at all times?
lp::Packet::wireDecodeis the only function invokingBlock::parse().lp::Packet::wireEncodesimply invokesBlock::encode().- All other functions assume
block.elements()reflects the truth, and ensureblock.elements()is up to date after the operation.
Updated by Eric Newberry almost 7 years ago
- Status changed from Code review to Closed
Updated by Eric Newberry almost 7 years ago
Just realized that NFD compilation will fail until https://gerrit.named-data.net/#/c/3848 is merged due to LpReliability's use of the deleted wireEncode function.
Updated by Junxiao Shi almost 7 years ago
Change 3848 probably needs more time. I've incorporated a quick fix in https://gerrit.named-data.net/3981 patchset3, which is already fixing breaking changes after merging https://gerrit.named-data.net/3979.