From 05ee550fce9918e7039aeb3dbc3fec72deb9df9a Mon Sep 17 00:00:00 2001 From: oneiric Date: Sat, 17 Mar 2018 02:47:16 +0000 Subject: [PATCH 1/2] AddressBook: store only unique addresses New setter for in-memory address book to insert unique entries. Refactor functions that store addresses to use the new setter. Refs #835 --- src/client/address_book/impl.cc | 54 ++++++++++++++++++++++++++------- src/client/address_book/impl.h | 11 ++++++- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/client/address_book/impl.cc b/src/client/address_book/impl.cc index 39d8e36d..78b28cc7 100644 --- a/src/client/address_book/impl.cc +++ b/src/client/address_book/impl.cc @@ -1,5 +1,5 @@ /** // - * Copyright (c) 2013-2017, The Kovri I2P Router Project // + * Copyright (c) 2013-2018, The Kovri I2P Router Project // * // * All rights reserved. // * // @@ -228,6 +228,7 @@ void AddressBook::DownloadSubscription() { << "AddressBook: picking random subscription from total publisher count: " << publisher_count; // Pick a random publisher to subscribe from + // TODO(oneiric): download all subscriptions not already stored auto publisher = kovri::core::RandInRange32(0, publisher_count - 1); m_SubscriberIsDownloading = true; try { @@ -295,6 +296,7 @@ bool AddressBook::SaveSubscription( LOG(debug) << "AddressBook: processing " << addresses.size() << " addresses"; // Stream may be a file or downloaded stream. // Regardless, we want to write/overwrite the subscription file. + // TODO(oneiric): save unique entries to userhosts.txt if (file_name.empty()) // Use default filename if none given. file_name = (core::GetPath(core::Path::AddressBook) / GetDefaultSubscriptionFilename()).string(); LOG(debug) << "AddressBook: opening subscription file " << file_name; @@ -307,11 +309,20 @@ bool AddressBook::SaveSubscription( for (auto const& address : addresses) { const std::string& host = address.first; const auto& ident = address.second; - // Write/overwrite Hostname=Base64Address pairing to subscription file - file << host << "=" << ident.ToBase64() << '\n'; // TODO(anonimal): this is not optimal, especially for large subscriptions - // Add to address book - m_Storage->AddAddress(ident); - m_Addresses[host] = ident.GetIdentHash(); // TODO(anonimal): setter? + try + { + // Only stores subscription lines for addresses not already loaded + InsertAddress(host, ident.GetIdentHash()); + // Write/overwrite Hostname=Base64Address pairing to subscription file + // TODO(anonimal): this is not optimal, especially for large subscriptions + file << host << "=" << ident.ToBase64() << '\n'; + m_Storage->AddAddress(ident); + } + catch (...) + { + m_Exception.Dispatch(__func__); + continue; + } } // Flush subscription file file << std::flush; @@ -319,10 +330,8 @@ bool AddressBook::SaveSubscription( m_Storage->Save(m_Addresses); m_SubscriptionIsLoaded = true; } - } catch (const std::exception& ex) { - LOG(error) << "AddressBook: exception in " << __func__ << ": " << ex.what(); } catch (...) { - LOG(error) << "AddressBook: unknown exception in " << __func__; + m_Exception.Dispatch(__func__); } return m_SubscriptionIsLoaded; } @@ -341,6 +350,7 @@ AddressBook::ValidateSubscription(std::istream& stream) { //const std::string alpha = "abcdefghijklmnopqrstuvwxyz"; // TODO(unassigned): expand when we want to venture beyond the .i2p TLD // TODO(unassigned): IDN ccTLDs support? + // TODO(unassigned): investigate alternatives to regex (maybe Boost.Spirit?) std::regex regex("(?=^.{1,253}$)(^(((?!-)[a-zA-Z0-9-]{1,63})|((?!-)[a-zA-Z0-9-]{1,63}\\.)+[a-zA-Z]+[(i2p)]{2,63})$)"); try { // Read through stream, add to address book @@ -441,6 +451,28 @@ std::unique_ptr AddressBook::GetLoadedAddressIdent return nullptr; } +void AddressBook::InsertAddress( + const std::string& host, + const kovri::core::IdentHash& address) +{ + try + { + // Ensure address book only inserts unique entries + if (!m_Addresses.empty()) + { + for (const auto& entry : m_Addresses) + if (entry.second == address) + throw std::runtime_error("AddressBook: address already loaded"); + } + m_Addresses[host] = address; + } + catch (...) + { + m_Exception.Dispatch(__func__); + throw; + } +} + // Used only by HTTP Proxy void AddressBook::InsertAddressIntoStorage( const std::string& address, @@ -450,11 +482,11 @@ void AddressBook::InsertAddressIntoStorage( { kovri::core::IdentityEx ident; ident.FromBase64(base64); + const auto& ident_hash = ident.GetIdentHash(); + InsertAddress(address, ident_hash); if (!m_Storage) m_Storage = GetNewStorageInstance(); m_Storage->AddAddress(ident); - const auto& ident_hash = ident.GetIdentHash(); - m_Addresses[address] = ident_hash; LOG(info) << "AddressBook: " << address << "->" << kovri::core::GetB32Address(ident_hash) << " added"; diff --git a/src/client/address_book/impl.h b/src/client/address_book/impl.h index 5396825a..7789fbf8 100644 --- a/src/client/address_book/impl.h +++ b/src/client/address_book/impl.h @@ -1,5 +1,5 @@ /** // - * Copyright (c) 2013-2017, The Kovri I2P Router Project // + * Copyright (c) 2013-2018, The Kovri I2P Router Project // * // * All rights reserved. // * // @@ -107,9 +107,18 @@ class AddressBook : public AddressBookDefaults { return m_SharedLocalDestination; } + /// @brief Insert address into in-memory storage + /// @param host Human-readable hostname to insert + /// @param address Hash of address to insert + /// @notes Throws if host or address are duplicates + void InsertAddress( + const std::string& host, + const kovri::core::IdentHash& address); + /// @brief Inserts address into address book from HTTP Proxy jump service /// @param address Const reference to human-readable address /// @param base64 Const reference to Base64 address + // TODO(oneiric): remove after separating HTTP Proxy from Address Book void InsertAddressIntoStorage( const std::string& address, const std::string& base64); From a6734661bfe8310497268e71de003f9c6b30bfb9 Mon Sep 17 00:00:00 2001 From: oneiric Date: Thu, 22 Mar 2018 04:57:54 +0000 Subject: [PATCH 2/2] Tests: Address Book unit-test checks unique entry New test case to ensure address book only inserts unique entries. New test fixture struct to convert a subscription line into an address book entry. Refs #835 --- tests/unit_tests/client/address_book/impl.cc | 59 +++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/client/address_book/impl.cc b/tests/unit_tests/client/address_book/impl.cc index 1796abb7..39000c30 100644 --- a/tests/unit_tests/client/address_book/impl.cc +++ b/tests/unit_tests/client/address_book/impl.cc @@ -1,5 +1,5 @@ /** // - * Copyright (c) 2015-2017, The Kovri I2P Router Project // + * Copyright (c) 2015-2018, The Kovri I2P Router Project // * // * All rights reserved. // * // @@ -44,6 +44,44 @@ /// @class SubscriptionFixture struct SubscriptionFixture { + struct AddressBookEntry + { + public: + explicit AddressBookEntry(const std::string& subscription_line) + { + try + { + std::size_t pos = subscription_line.find('='); + if (pos == std::string::npos) + throw std::runtime_error( + "AddressBookEntry: invalid subscription line"); + m_Host = subscription_line.substr(0, pos++); + kovri::core::IdentityEx ident; + ident.FromBase64(subscription_line.substr(pos)); + m_Address = ident.GetIdentHash(); + } + catch (...) + { + kovri::core::Exception ex; + ex.Dispatch(__func__); + throw; + } + } + + const std::string& host() const + { + return m_Host; + } + + const kovri::core::IdentHash& address() const + { + return m_Address; + } + + private: + std::string m_Host; ///< Human-readable I2P hostname + kovri::core::IdentHash m_Address; ///< I2P address hash + }; /// @brief Validates given lines as proven addressbook host/address pairs /// @param lines Lines to validate @@ -170,4 +208,21 @@ BOOST_AUTO_TEST_CASE(PGPClearSign) { // TODO(unassigned): more cases? -BOOST_AUTO_TEST_SUITE_END() +BOOST_AUTO_TEST_CASE(RejectDuplicateEntry) +{ + // Ensure valid subscription line creates an entry + BOOST_CHECK_NO_THROW(AddressBookEntry entry(subscription.front())); + + AddressBookEntry entry(subscription.front()); + // Ensure valid entry is inserted + BOOST_CHECK_NO_THROW(book.InsertAddress(entry.host(), entry.address())); + // Ensure address book throws for duplicate host + BOOST_CHECK_THROW( + book.InsertAddress(entry.host(), entry.address()), std::runtime_error); + // Ensure address book throws for duplicate address + BOOST_CHECK_THROW( + book.InsertAddress("unique." + entry.host(), entry.address()), + std::runtime_error); +} + +BOOST_AUTO_TEST_SUITE_END()