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
Hello Miroslav,
On Sun, Jan 26, 2020 at 2:57 AM Miroslav Zagorac wrote:
> i think that patch does not correct the bug in that function because in
> the loop above the variable i and pointer d increase at the same time.
>
> This means that *d should be written instead of d[i].
that is very true,
William,
Am 26.01.20 um 18:41 schrieb William Dauchy:
> int dns_hostname_validation(const char *string, char **err)
> {
> - const char *c, *d;
> int i;
Consider moving this into the `while` loop to reduce the scope of `i`.
> + while (*string && *string != '.' && i <
William,
Am 26.01.20 um 19:34 schrieb William Dauchy:
> On Sun, Jan 26, 2020 at 7:20 PM Tim Düsterhus wrote:
>>> int i;
>>
>> Consider moving this into the `while` loop to reduce the scope of `i`.
>
> I'm not against doing this when this is a block condition, but for a
> loop, I find it a
On 26 Jan 18:46, William Dauchy wrote:
> On Sun, Jan 26, 2020 at 7:33 PM Tim Düsterhus wrote:
> > Backport information are missing (without looking up that commit). 1.8+
> > it is.
>
> Thanks. Could be nice to change a bit these rules; indeed, when the
> `Fixes` tag is present, it's very easy to
Hello,
On Sun, 26 Jan 2020 at 20:11, William Dauchy wrote:
> > > The explanation of the user-visible impact and the need for
> > backporting to stable branches or not are MANDATORY.
>
> Yes; I was simply challenging that, as it is also open to mistakes to
> write in commit message to which
On Sun, Jan 26, 2020 at 11:06 PM Lukas Tribus wrote:
> Just because commit 456 fixes something that has been introduced with
> commit 123 DOES NOT mean we backport 456 to all the branches that 123
> was committed to - instead we backport 456 to a certain branch if it
> *actually* makes sense to
Ilya,
Am 26.01.20 um 19:06 schrieb Илья Шипицин:
> such things are fragile. once fixed, they can silently break during
> further refactoring.
The function is fairly self contained. I don't expect there to be any
need for refactoring. Also the previous version was fairly elaborate for
a simple
On Sun, Jan 26, 2020 at 7:33 PM Tim Düsterhus wrote:
> Backport information are missing (without looking up that commit). 1.8+
> it is.
Thanks. Could be nice to change a bit these rules; indeed, when the
`Fixes` tag is present, it's very easy to ask git in which tag this
was introduced; so in my
On Sun, Jan 26, 2020 at 8:17 PM Julien Pivotto wrote:
> That will not work with cherry-picked commits.
that's what I nuanced in the previous email you quoted indeed.
--
William
William,
Willy,
Am 26.01.20 um 19:28 schrieb Tim Düsterhus:
>> Fixes: 7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
>> protocols are mixed")
One more thing:
Backport information are missing (without looking up that commit). 1.8+
it is.
> Oh no, I recognize that commit. That's
On Sun, Jan 26, 2020 at 7:56 PM Tim Düsterhus wrote:
> Yes, and because this is so easy to look up I simply add both the commit
> and the first version to my commit message to save the person doing the
> backporting the brain cycles. Backporting appears to mostly be a bulk
> process and I can
On Sun, Jan 26, 2020 at 7:08 PM Илья Шипицин wrote:
> such things are fragile. once fixed, they can silently break during further
> refactoring.
> on other hand, such functions are good candidates to write unit tests.
I considered it but to my knowledge, this is currently not possible
with
Thanks for your review Tim
On Sun, Jan 26, 2020 at 7:20 PM Tim Düsterhus wrote:
> > int i;
>
> Consider moving this into the `while` loop to reduce the scope of `i`.
I'm not against doing this when this is a block condition, but for a
loop, I find it a bit dirty and confusing.
> For
hostname were limited to 62 char, which is not RFC1035 compliant;
- the parsing loop should stop when above max label char
- fix len label test where d[i] was wrongly used
- simplify the whole function to avoid using two extra char* variable
this should fix github issue #387
Signed-off-by:
triggered by coverity; src_port is set earlier.
this should fix github issue #467
Fixes: 7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
protocols are mixed")
Signed-off-by: William Dauchy
---
src/connection.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
such things are fragile. once fixed, they can silently break during
further refactoring.
on other hand, such functions are good candidates to write unit tests.
maybe we should consider things like cmocka ?
вс, 26 янв. 2020 г. в 22:44, William Dauchy :
> hostname were limited to 62 char, which
вс, 26 янв. 2020 г. в 23:12, William Dauchy :
> On Sun, Jan 26, 2020 at 7:08 PM Илья Шипицин wrote:
> > such things are fragile. once fixed, they can silently break during
> further refactoring.
> > on other hand, such functions are good candidates to write unit tests.
>
> I considered it but
William,
Am 26.01.20 um 19:52 schrieb William Dauchy:
> hostname were limited to 62 char, which is not RFC1035 compliant;
> - the parsing loop should stop when above max label char
> - fix len label test where d[i] was wrongly used
> - simplify the whole function to avoid using two extra char*
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 =
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
triggered by coverity; src_port is set earlier.
this should fix github issue #467
Fixes: 7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
protocols are mixed")
Signed-off-by: William Dauchy
---
src/connection.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
William,
Am 26.01.20 um 19:06 schrieb William Dauchy:
> Fixes: 7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
> protocols are mixed")
Oh no, I recognize that commit. That's mine :-(
> - hdr->addr.ip6.src_port = ((struct sockaddr_in
> *)src)->sin_port;
William,
Am 26.01.20 um 19:46 schrieb William Dauchy:
> On Sun, Jan 26, 2020 at 7:33 PM Tim Düsterhus wrote:
>> Backport information are missing (without looking up that commit). 1.8+
>> it is.
>
> Thanks. Could be nice to change a bit these rules; indeed, when the
> `Fixes` tag is present,
hostname were limited to 62 char, which is not RFC1035 compliant;
- the parsing loop should stop when above max label char
- fix len label test where d[i] was wrongly used
- simplify the whole function to avoid using two extra char* variable
this should fix github issue #387
Signed-off-by:
25 matches
Mail list logo
