Page MenuHomePhabricator
Differential D4418 Diff 15314 src/seeder/test/seeder_tests.cpp

Changeset View

Standalone View

View Options

src/seeder/test/seeder_tests.cpp

// Copyright (c) 2019 The Bitcoin developers // Copyright (c) 2019 The Bitcoin developers
// Distributed under the MIT software license, see the accompanying // Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php. // file COPYING or http://www.opensource.org/licenses/mit-license.php.
#define BOOST_TEST_MODULE Bitcoin Seeder Test Suite #define BOOST_TEST_MODULE Bitcoin Seeder Test Suite
#include <seeder/dns.h>
#include <array>
#include <string>
#include <vector>
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
BOOST_AUTO_TEST_SUITE(seeder_tests) BOOST_AUTO_TEST_SUITE(seeder_tests)
BOOST_AUTO_TEST_CASE(test_stub) { static const int QUERY_NAME_BUFFER_LENGTH = 256;
BOOST_CHECK_EQUAL(true, true); static const int QUERY_TYPE_SIZE = 2;
static const int QUERY_CLASS_SIZE = 2;
static const uint16_t QUERY_CLASS_IN = 1;
static const uint16_t QUERY_TYPE_A = 1;
static const uint8_t END_OF_NAME_FIELD = 0;
static const size_t MAX_LABEL_LENGTH = 63;
// Builds the question section of a DNS query
std::vector<uint8_t> CreateDNSQuestion(const std::string &queryName) {
std::vector<uint8_t> dnsQuestion;
size_t i = 0;
size_t lastPeriod = 0;
while (i < queryName.size()) {
if (queryName[i] == '.') {
// Push the length of the label and then the label
dnsQuestion.push_back(i - lastPeriod);
while (lastPeriod < i) {
dnsQuestion.push_back(queryName[lastPeriod]);
lastPeriod++;
}
lastPeriod = i + 1;
}
i++;
}
// Push the length of the label and then the label
dnsQuestion.push_back(i - lastPeriod);
while (lastPeriod < i) {
dnsQuestion.push_back(queryName[lastPeriod]);
lastPeriod++;
}
dnsQuestion.push_back(END_OF_NAME_FIELD);
dnsQuestion.push_back(QUERY_TYPE_A);
dnsQuestion.push_back(QUERY_CLASS_IN);
return dnsQuestion;
}
BOOST_AUTO_TEST_CASE(parse_name_happy_path) {
const std::string messageQueryName = "www.mydomain.com";
std::vector<uint8_t> dnsQuestion = CreateDNSQuestion(messageQueryName);
std::array<char, QUERY_NAME_BUFFER_LENGTH> queryName;
queryName.fill(0);
const uint8_t *messageBegin = dnsQuestion.data();
// +1 for the last octet ending the name field
const size_t messageEndIndex =
messageQueryName.size() + 1 + QUERY_TYPE_SIZE + QUERY_CLASS_SIZE;
deadalnixUnsubmitted
Not Done
Inline Actions

Wouldn't that be less dangerous to use the size of the question, or at least assert that all these computed size are compatible with each others?

deadalnix: Wouldn't that be less dangerous to use the size of the question, or at least assert that all…
int ret = parse_name(&messageBegin, messageBegin + messageEndIndex,
dnsQuestion.data(), queryName.data(),
QUERY_NAME_BUFFER_LENGTH);
BOOST_CHECK_EQUAL(ret, 0);
BOOST_CHECK_EQUAL(queryName.data(), messageQueryName);
}
// Test for insufficient output buffer size
BOOST_AUTO_TEST_CASE(parse_name_insufficient_output_buffer_size) {
const std::string messageQueryName = "www.mydomain.com";
std::vector<uint8_t> dnsQuestion = CreateDNSQuestion(messageQueryName);
std::array<char, QUERY_NAME_BUFFER_LENGTH> queryName;
queryName.fill(0);
const uint8_t *messageBegin = dnsQuestion.data();
// +1 for the last octet ending the name field
const size_t messageEndIndex =
messageQueryName.size() + 1 + QUERY_TYPE_SIZE + QUERY_CLASS_SIZE;
// The size of the buffer being written to is 1 octect too small
int ret = parse_name(&messageBegin, messageBegin + messageEndIndex,
dnsQuestion.data(), queryName.data(),
messageQueryName.size());
BOOST_CHECK_EQUAL(ret, -2);
deadalnixUnsubmitted
Not Done
Inline Actions

The only thing that seems to be different from the previous test to get that result is the size passed to the function above as last parameter. Reviewing this feels a lot like playing where is Waldo, which is a sure sign the code is unreadable.

You test case should read like "We test X with parameters A, B an C, we expect the result to be Y" with various sets of A, B and C and Y.

I did not review subsequent tests because it would literally takes less time to actually write them than review this.

deadalnix: The only thing that seems to be different from the previous test to get that result is the size…
BOOST_CHECK_EQUAL(queryName.data(),
messageQueryName.substr(0, messageQueryName.size() - 1));
}
// Test for premature end of input buffer
BOOST_AUTO_TEST_CASE(parse_name_premature_end_of_input_buffer) {
const std::string messageQueryName = "www.mydomain.com";
std::vector<uint8_t> dnsQuestion = CreateDNSQuestion(messageQueryName);
std::array<char, QUERY_NAME_BUFFER_LENGTH> queryName;
queryName.fill(0);
const uint8_t *messageBegin = dnsQuestion.data();
// The end index pointer for the DNS message buffer passed is located two
// octets away from the beginning
int ret = parse_name(&messageBegin, messageBegin + 2, dnsQuestion.data(),
queryName.data(), QUERY_NAME_BUFFER_LENGTH);
BOOST_CHECK_EQUAL(ret, -1);
BOOST_CHECK_EQUAL(queryName.data(), messageQueryName.substr(0, 1));
}
// Test for when name field is too long
BOOST_AUTO_TEST_CASE(parse_name_field_name_too_long) {
std::string tooLongQName = "www.";
for (size_t i = 0; i < MAX_LABEL_LENGTH + 1; i++) {
tooLongQName += 'a';
}
tooLongQName += ".com";
std::vector<uint8_t> dnsQuestion = CreateDNSQuestion(tooLongQName);
std::array<char, QUERY_NAME_BUFFER_LENGTH> queryName;
queryName.fill(0);
const uint8_t *messageBegin = dnsQuestion.data();
size_t messageEndIndex =
tooLongQName.size() + 1 + QUERY_TYPE_SIZE + QUERY_CLASS_SIZE;
int ret = parse_name(&messageBegin, messageBegin + messageEndIndex,
dnsQuestion.data(), queryName.data(),
QUERY_NAME_BUFFER_LENGTH);
BOOST_CHECK_EQUAL(ret, -1);
BOOST_CHECK_EQUAL(queryName.data(), tooLongQName.substr(0, 4));
} }
BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END()