Re: [PATCH v2] BUG/MINOR: dns: allow 63 char in hostname

[William Dauchy]

Hello Tim,

On Sun, Jan 26, 2020 at 1:39 PM Tim Düsterhus  wrote:
> I wonder if we should move the `i++` down to above the `d++`. `i` is not
> being used in the loop body and the only way to exit the loop is the
> `return`. Moving it down has the benefit that it's not as easy to miss
> that both are incremented within the loop (like you did in your first
> patch).
>
> Or we could get rid of either `i` or `d` entirely and replace it by `(d
> - c)` or `c[i]` respectively.

In fact I've sent v1 by going too fast on my tests, then I realised we
could simplify a lot of things potentially, but would make the patch
easier to be read in two patches, so I sent v2. However as I see you
also agree on a few points I thought about, I will send v3 with a
simplified version of the function.

Thanks,
-- 
William

Re: [PATCH v2] BUG/MINOR: dns: allow 63 char in hostname

[Tim Düsterhus]

William,

Am 26.01.20 um 11:07 schrieb William Dauchy:
> diff --git a/src/dns.c b/src/dns.c
> index eefd8d0dc..dccd0498c 100644
> --- a/src/dns.c
> +++ b/src/dns.c
> @@ -1484,7 +1484,7 @@ int dns_hostname_validation(const char *string, char 
> **err)
>   d = c;
>  
>   i = 0;
> - while (*d != '.' && *d && i <= DNS_MAX_LABEL_SIZE) {
> + while (*d != '.' && *d && i < DNS_MAX_LABEL_SIZE) {
>   i++;

I wonder if we should move the `i++` down to above the `d++`. `i` is not
being used in the loop body and the only way to exit the loop is the
`return`. Moving it down has the benefit that it's not as easy to miss
that both are incremented within the loop (like you did in your first
patch).

Or we could get rid of either `i` or `d` entirely and replace it by `(d
- c)` or `c[i]` respectively.

Best regards
Tim Düsterhus

[PATCH v2] BUG/MINOR: dns: allow 63 char in hostname

[William Dauchy]

hostname were limited to 62 char, which is not RFC1035 compliant;
- the loop should stop when above max char
- fix resulting test where d[i] was wrongly used
(also simplify brackets for readability)

this should github issue #387

Signed-off-by: William Dauchy 
---
 src/dns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dns.c b/src/dns.c
index eefd8d0dc..dccd0498c 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1484,7 +1484,7 @@ int dns_hostname_validation(const char *string, char 
**err)
d = c;
 
i = 0;
-   while (*d != '.' && *d && i <= DNS_MAX_LABEL_SIZE) {
+   while (*d != '.' && *d && i < DNS_MAX_LABEL_SIZE) {
i++;
if (!((*d == '-') || (*d == '_') ||
  ((*d >= 'a') && (*d <= 'z')) ||
@@ -1497,7 +1497,7 @@ int dns_hostname_validation(const char *string, char 
**err)
d++;
}
 
-   if ((i >= DNS_MAX_LABEL_SIZE) && (d[i] != '.')) {
+   if (i >= DNS_MAX_LABEL_SIZE && *d != '.') {
if (err)
*err = DNS_LABEL_TOO_LONG;
return 0;
-- 
2.24.1