Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Review cpp-netlib for full implementation #155

Closed
1 task done
anonimal opened this issue Mar 5, 2016 · 4 comments
Closed
1 task done

Review cpp-netlib for full implementation #155

anonimal opened this issue Mar 5, 2016 · 4 comments

Comments

@anonimal
Copy link
Collaborator

anonimal commented Mar 5, 2016

By submitting this issue, I confirm the following:

  • I have read and understood the contributor guide.
  • I have checked that the issue I am reporting can be replicated or that the feature I am suggesting is not present.
  • I have checked opened or recently closed pull requests for existing solutions/implementations to my issue/suggestion.

Place an X inside the bracket to confirm

  • I confirm.

In today's meeting, we agreed to move on cpp-netlib pending more review and provided that we are able to make certain adjustments to fit our needs. Excerpt from meeting:

2016-03-05  * anonimal repasting
2016-03-05  &anonimal 2016-03-05 18:45:33 &anonimal Considering the importance of auto-update,
2016-03-05  &anonimal 2016-03-05 18:45:42 &anonimal I don't want to go cheap on any library so,
2016-03-05  &anonimal 2016-03-05 18:45:56 &anonimal maybe the flexibility of cpp-netlib is worth the cost.
2016-03-05  &anonimal 2016-03-05 18:47:06 &anonimal Or we can just keep doing things by hand and do extensive review over time.
2016-03-05  &anonimal 2016-03-05 18:47:23 &anonimal But that takes time away from other things.
2016-03-05  &anonimal 2016-03-05 18:49:02 &anonimal EinMByte: are you able to vote now or do you need more time?
2016-03-05  &anonimal 2016-03-05 18:58:08 &anonimal EinMByte: last message you received?
2016-03-05  +EinMByte I think auto-update would be easier if we bundle a library
2016-03-05  +EinMByte that is, anything static is easy to auto-update because we don't have to require any updated dependencies
2016-03-05  +EinMByte I'm looking at http::client
2016-03-05  +EinMByte It seems like there is a template parameter called Tag that can be changed
2016-03-05  +EinMByte The default is UDP
2016-03-05  +EinMByte anonimal: https://github.com/cpp-netlib/cpp-netlib/blob/master/boost/network/protocol/http/tags.hpp
2016-03-05  &anonimal Yep, so do you vote yay or nay for full implementation of cpp-netlib or do you want to review more?
2016-03-05  +EinMByte I vote yes on the condition that we can create our own tag
2016-03-05  &anonimal I'm fine with more review now that we know what lib to focus on.
2016-03-05  +EinMByte If that's not possible, the choice is much harder imho
2016-03-05  +EinMByte so let's say more research needed?
2016-03-05  &anonimal Ok, how about we open a ticket to review ability to create custom tag. If that passes, then we can move onto implementing.
2016-03-05  +EinMByte ok
@anonimal
Copy link
Collaborator Author

anonimal commented Mar 5, 2016

Tried to assign to two people, forgot that doesn't work 😄

@anonimal anonimal mentioned this issue Mar 26, 2016
15 tasks
anonimal referenced this issue in anonimal/kovri Apr 11, 2016
* C++11 (minimum) refactor
* Create/rewrite appropriate classes
  - Improve class Reseed
  - Create classes SU3, ZIP, X509, and HTTP
  - Refactoring: move the mess of local variables into a POD type
  - Use spec-identifiable constants instead of raw numbers
  - Move class ZIP to core/util/ZIP.{h,cpp}
  - Move class X509 to core/crypto/X509.{h,cpp}
* Design rewrite
  - Create better reseed abstraction
    - See constructors, design, and interface of class
      Reseed/SU3/ZIP/X509
  - Separate parsing:
    - Class SU3 parsing from X.509/signature parsing and from class ZIP
      parsing
  - Rewrite/refactor related class NetDb code
  - Refactor: get rid of -1 return values, use bool and adjust related
    code
* Create/implement stream abstraction
  - Stream wrapper for strongly-typed classes
* Abstract CryptoPP from class ZIP and class X509
  - Pimpl ZIP decompression
  - Pimpl X.509 and separate cert processing from decoding
  - Ensure uncaught exceptions are caught
  - Cleanup pimpl-related directory structure
* Feature: manual reseed: create/implement an overloaded --reseed-from
  run-time switch
  - Handles reseeding from file
  - Handles reseeding from specified URL
* Feature: create/implement --reseed-skip-ssl-check run-time switch
  - Allows connecting to servers with certificates not shipped with
    Kovri (such as a local server)
  - Users can still put their self-signed cert in
    KOVRI_DATA_DIR/certificates/su3
    and skip this switch if desired
* HTTP: minor design refactor to accommodate new class Reseed design
  - Download function stores results in member stream, returns bool
  - HTTP response is stored in member variable
  - Logic design refactor to fix erroneous error response and help with
    debugging
* Spec review
  - Ensure that SU3 implementation meets requirements and provides a
    minimal interface to implement future content-types and/or specifications
    (ex. for auto-update or news feed)
  - Ensure that ZIP meets minimum requirements for our use-case
* Create secure sanity checks
* Create unit tests (referencing #7)
  - Tests for class SU3, ZIP, and X509
  - Cleanup unit-test directory structure and rename appropriate files
  - Adjust CMake accordingly
* Document code
  - Extensive documentation where possible
* Resolve any preexisting TODO's
* General improvements and rewrites
@anonimal anonimal mentioned this issue Jul 9, 2016
@anonimal
Copy link
Collaborator Author

anonimal commented Jul 9, 2016

Ah, GitHub finally implemented multiple assignees to issues. Assigning both myself and @EinMByte.

@anonimal
Copy link
Collaborator Author

anonimal commented Jul 15, 2016

cpp-netlib's master branch presently fails to build on Arch because of deprecated Asio/SSLv3 symbols (but this is known). Building from either 0.13-release, 0.12-release, or 0.12-devel is successful. Building from tagged cpp-netlib-0.12.0-rc2 still fails.

anonimal added a commit to anonimal/kovri that referenced this issue Jul 21, 2016
@anonimal anonimal mentioned this issue Aug 9, 2016
8 tasks
anonimal added a commit to anonimal/kovri that referenced this issue Sep 5, 2016
Major:
- Implementation improvements
  * HTTP: download impl
  * HTTP: URI handling impl
  * HTTP: response header handling for clearnet
  * AddressBook: unhook in-net HTTP impl (now in class HTTP)
  * Add documentation

Medium:
- If default subscription file does not exist,
  create/save after downloading from publisher

Minor:
- Adjust related reseed and tunnel code for class HTTP
- Update variables names to accurately reflect purpose
- Update log messages for accuracy
- Update address book flow narrative
- Update/improve HTTP unit-test

References:
- monero-project#305 (parent ticket)
- monero-project#168 and monero-project#155 (both in terms of HTTP/S)
- monero-project#154 and monero-project#48 (we now have a multi-purpose HTTP/S download impl)

Notes:
- My apologies for not keeping this commit atomic.
  Bigger design work can be harder to keep atomic
  (especially if one wants new features to work before committing)
- This commit has been successfully tested and all unit-tests pass
@anonimal
Copy link
Collaborator Author

As noted in #718, cpp-netlib development is not nearly where it should be (in terms of activity) for me to gain enough confidence in the future of their library. Though the abstract messaging potential is there, and the library has great potential, I don't want to put any potential risk for backtracking onto kovri's future.

Our current impl of cpp-netlib is suitable to our present needs and need not be expanded at this present time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants