[BUG] DNS does not resolve long names

[marcusb]

Describe the bug

When ipconfigUSE_DNS_CACHE == 1, the DNS module fails to resolve domain names longer than ipconfigDNS_CACHE_NAME_LENGTH (default 16). The lookup is rejected by this line.

To Reproduce

• Configure project with ipconfigUSE_DNS_CACHE 1 and ipconfigDNS_CACHE_NAME_LENGTH 16
• Call FreeRTOS_gethostbyname() with a name longer than 16 bytes.
• The call returns 0

Expected behavior

The name resolution should succeed. The name limit should apply only to caching the response, but the DNS resolution should still happen.

[kstribrnAmzn]

Thanks for reporting this bug - I took at look through the code and you're right, the behavior is different from the documentation! I'm not as familiar with this code base so I'll notify a teammate who is. They should be able to work on it in the coming days.

If you'd like a fix faster, please raise a PR which we can take a look at.

[kstribrnAmzn]

For future implementors it looks like this line prevents the DNS query for the IP address when the name is longer than the DNS cache name length. It appears the check of ( xLengthOk != pdFALSE ) should be move to this line which occurs right before the cache isn't used.

The part I'm not familiar with is how cache entries are added. The behavior should be to look up the IP address and cache after this line (?).

[tony-josi-aws]

@marcusb

Currently the expectation is that all the DNS resolutions will be cached if ipconfigUSE_DNS_CACHE is enabled; for that to happen, we need all hostnames to have their length, uxLength <= ipconfigDNS_CACHE_NAME_LENGTH. The documentation of ipconfigDNS_CACHE_NAME_LENGTH also doesn't say that we cache only names lower than its value; instead, it says:

The maximum number of characters a DNS host name can take, including the NULL terminator.

Though this is currently the expected behavior, the macro naming is confusing, and it also makes sense to have caching optional based on ipconfigDNS_CACHE_NAME_LENGTH not the other way round, as @kstribrnAmzn suggested.

If you wish, please feel free to raise a PR.

[DakshitBabbar]

We are closing this issue for the time being. Please don't hesitate to reopen it if additional input or follow-up is required.
Thanks.

[tony-josi-aws]

Hi @marcusb

We have updated the website docs related to the DNS cache name length, specifying this behavior:

• ipconfigUSE_DNS_CACHE
• FreeRTOS_gethostbyname()
• FreeRTOS_gethostbyname_a()

Thank you