Project

General

Profile

Feature #3741

Rewrite ndn::io without CryptoPP

Added by Junxiao Shi almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Utils
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
3.00 h

Description

Rewrite ndn::io::load and ndn::io::save so that they do not rely on CryptoPP.

#1

Updated by Junxiao Shi almost 4 years ago

  • Status changed from New to In Progress
  • Assignee set to Junxiao Shi
#2

Updated by Junxiao Shi almost 4 years ago

https://gerrit.named-data.net/3087 improves test coverage.

#3

Updated by Junxiao Shi almost 4 years ago

  • % Done changed from 0 to 40
#4

Updated by Junxiao Shi almost 4 years ago

  • % Done changed from 40 to 60

https://gerrit.named-data.net/3104 implements ndn::io with ndn::security::transform, and adds Doxygen.

#5

Updated by Junxiao Shi almost 4 years ago

I'll wait for the last three commits to be merged before doing the following related changes:

  • keep wireEncode and wireDecode in the template, and move transform part to helper functions in .cpp
  • require WireEncodable and WireDecodable concepts for T, and use T(const Block&) constructor instead of wireDecode method
  • do not require T::Error type
#6

Updated by Junxiao Shi almost 4 years ago

  • % Done changed from 60 to 70

https://gerrit.named-data.net/3111 moves transform part of io::load and io::save into io::loadBlock and io::saveBlock.

#7

Updated by Junxiao Shi almost 4 years ago

NFD Mgmt/TestCommandAuthenticator/BadConfig/CertUnreadable test case fails to terminate since commit:f2515a772b4a1c401927a748371cb36b94022caf is merged.
In that test case, io::load is used to read from a non-existent file.
In this situation, std::ifstream constructor sets failbit but not badbit, but unlike CryptoPP, transform::StreamSource does not properly handle this situation.

Test case Util/TestIo/LoadFileNotReadable is meant to catch this situation.
However, this test case creates a directory at filename, and open(dirName) can be successful on many platforms.

https://gerrit.named-data.net/3113 corrects the condition within StreamSource, and also corrects the setup in Util/TestIo/LoadFileNotReadable test case.

#8

Updated by Junxiao Shi almost 4 years ago

https://gerrit.named-data.net/3111 breaks NFD and ChronoChat because they rely on some #includes that are no longer there.

https://travis-ci.org/yoursunny/ndn-cxx-breaks/builds/153662514

https://gerrit.named-data.net/3120 adds proper #includes to ChronoChat.

I'll patch NFD later.

#9

Updated by Junxiao Shi almost 4 years ago

There's another regression exposed in ChronoChat http://jenkins.named-data.net/job/ChronoChat/97/

The failing test case invokes io::load(is, BASE_64) where is contains a base64-encoded object all on one line.
It seems that io::load is not correctly handling this case, and that existing base64 test cases only cover very small objects.

I'll investigate this soon.

#10

Updated by Junxiao Shi almost 4 years ago

https://gerrit.named-data.net/3121 adds proper #include to NFD.

#11

Updated by Junxiao Shi almost 4 years ago

Potentially breaking change notice for https://gerrit.named-data.net/3111 is sent to ndn-lib nfd-dev.
Deadline is Aug 24. This is shorter than the usual 5-day period because transitive includes are not guaranteed public API.

#12

Updated by Alex Afanasyev almost 4 years ago

Junxiao Shi wrote:

There's another regression exposed in ChronoChat http://jenkins.named-data.net/job/ChronoChat/97/

The failing test case invokes io::load(is, BASE_64) where is contains a base64-encoded object all on one line.
It seems that io::load is not correctly handling this case, and that existing base64 test cases only cover very small objects.

I'll investigate this soon.

Unfortunately, openssl is not as flexible as CryptoPP. transform::base64Decode by default assumes there are separators, can be supplied 'false' for otherwise, but doesn't allow guessing whether input has separators or not.

#13

Updated by Junxiao Shi almost 4 years ago

https://gerrit.named-data.net/3137 solves note-9 regression.

I have also renamed BASE_64 to BASE64 which is more logical.
This requires a patch for nTorrent https://gerrit.named-data.net/3138.

#14

Updated by Junxiao Shi almost 4 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 70 to 100

https://gerrit.named-data.net/3149 adds concept checks and stops requiring T::Error.

#15

Updated by Junxiao Shi almost 4 years ago

https://gerrit.named-data.net/3126 BASE_64 renaming also needs a patch for ndns https://gerrit.named-data.net/3150.

Breaking change notice is sent to ndn-lib nfd-dev, deadline Sep 04.

#16

Updated by Junxiao Shi almost 4 years ago

  • Status changed from Code review to Closed

Also available in: Atom PDF