Skip to content

Fixes 16 #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aarond10 merged 4 commits into from
Oct 7, 2025
Merged

Fixes 16 #193

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
669dfe3
Warning fixes
baranyaib90 Sep 7, 2025
71d3065
Avoid division by zero
baranyaib90 Sep 11, 2025
0092cb5
clang tidy changes
baranyaib90 Sep 20, 2025
138b7ee
Replace atoi with strtoul
baranyaib90 Sep 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/dns_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size

// rough estimate to reach size limit
size_t answers = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER);
size_t answers_to_keep = (size_limit - DNS_HEADER_LENGTH) / (old_size / answers);
size_t answers_to_keep = ((size_limit - DNS_HEADER_LENGTH) * answers) / old_size;
Copy link
Owner

@aarond10 aarond10 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should simplify this whole function to something like this (nb: untested)?

while (*buflen > 0) {
    ares_status_t status = ares_dns_parse((const unsigned char *)buf, *buflen, 0, &dnsrec);
    if (status != ARES_SUCCESS) {
      WLOG("Failed to parse DNS response: %s", ares_strerror((int)status));
      return;
    }
    int32_t packet_len = *buflen;
    while (packet_len > size_limit) {
        size_t current_count = ares_dns_record_rr_cnt(dnsrec, ARES_DNS_SECTION_ANSWER);
        if (current_count <= 1) { // Stop if we are about to delete the last record
            ELOG("Cannot trim any more records. Still too large.");
            break;
        }

        // Get the last record
        const ares_dns_rr_t *last_record = ares_dns_record_rr_get_const(dns_record, ARES_DNS_SECTION_ANSWER, current_count - 1);
        if (last_record == NULL) {
            printf("Error finding last record.\n");
            break;
        }

        // Estimate the size of the record we are about to delete
        size_t size_to_remove = 10 + strlen(lat_record->name) + 1 + last_record->rdata_len;

        status = ares_dns_record_rr_del(dns_record, ARES_DNS_SECTION_ANSWER, current_count - 1);
        if (status != ARES_SUCCESS) {
            ELOG("Error deleting record: %s", ares_strerror(status));
            break;
        }
        
        // Subtract the estimated size from our total
        // Note: This is an approximation. The actual size reduction
        // might differ slightly due to name compression effects.
        packet_len -= size_to_remove;
    }
    status = ares_dns_record_write_packet(dnsrec, buf, buflen);
}
...

Copy link
Owner

@aarond10 aarond10 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this might be a good idea...

Copy link
Contributor Author

@baranyaib90 baranyaib90 Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I will check later more deeply, but this code looks to be AI generated with lots of mistakes:

This was my bigges issue with truncation: you can't know trough public API how large the record is that you intend to remove.
That is why I suppose that records are even sized (not true for TXT records obviously) and I'm using ratio-based estimation how much should be removed. If the DNS response would be still over the limit, I'm starting to aggressively reduce the size by dropping half of the records all the time until limit is reached or 1 remains (because even for TXT record, the maximum string limit is 255 char, so should fit in minimum 512).
I try not to waste CPU time by calling ares_dns_write() way too many times, but with current public API of c-ares there is no better option.
I agree, that current function is large and complicated, but at lest it seems to work and I would better not touch it until there is an issue with it.
Br, Balázs

answers_to_keep = answers_to_keep > 0 ? answers_to_keep : 1; // try to keep 1 answer

// remove answer records until fit size limit or running out of answers
Expand Down Expand Up @@ -196,6 +196,10 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size

void dns_server_respond(dns_server_t *d, struct sockaddr *raddr,
const char *dns_req, const size_t dns_req_len, char *dns_resp, size_t dns_resp_len) {
if (dns_resp_len < DNS_HEADER_LENGTH) {
WLOG("Malformed response received, invalid length: %u", dns_resp_len);
return;
}
if (dns_resp_len > DNS_SIZE_LIMIT) {
const uint16_t udp_size = get_edns_udp_size(dns_req, dns_req_len);
if (dns_resp_len > udp_size) {
Expand Down