QUIC-LB: Generating Routable QUIC Connection IDs

[Aleksandar Lazic]

Hi.

Have you seen this Draft?

https://datatracker.ietf.org/doc/draft-ietf-quic-load-balancers/

Because there are a lot of QUIC Drafts there and 2.2 is released it would be 
nice
to get some update about the QUIC state in HAProxy ;-).

https://datatracker.ietf.org/doc/search/?name=QUIC&activedrafts=on&rfcs=on

Best Regards

Aleks

[PATCH] suppress "return value is not checked" warnings

[Илья Шипицин]

Hello,

cleanup patch attached.


Ilya Shipitcin
From e5a49969d374e3e8e9da695dca48cb6fa82ca13d Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sun, 26 Jul 2020 15:01:10 +0500
Subject: [PATCH] CLEANUP: suppress coverity warnings

Coverity is not happy when return value is not examined. Those
warnings are already marked as "Intentional", let us explicitely
mark them with DISGUISE(..)
---
 include/haproxy/connection.h | 10 +-
 src/cli.c|  2 +-
 src/dns.c|  2 +-
 src/fd.c |  2 +-
 src/haproxy.c|  2 +-
 src/listener.c   |  2 +-
 src/log.c|  4 ++--
 src/mworker.c|  2 +-
 src/pipe.c   |  2 +-
 src/proto_sockpair.c |  4 ++--
 src/proto_tcp.c  | 14 +++---
 src/proto_udp.c  |  2 +-
 src/proto_uxst.c |  4 ++--
 src/session.c| 16 
 14 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h
index 05e85e1cb..c7eb7ab19 100644
--- a/include/haproxy/connection.h
+++ b/include/haproxy/connection.h
@@ -582,15 +582,15 @@ static inline void conn_set_tos(const struct connection *conn, int tos)
 
 #ifdef IP_TOS
 	if (conn->src->ss_family == AF_INET)
-		setsockopt(conn->handle.fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos));
+		DISGUISE(setsockopt(conn->handle.fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos)));
 #endif
 #ifdef IPV6_TCLASS
 	if (conn->src->ss_family == AF_INET6) {
 		if (IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6 *)conn->src)->sin6_addr))
 			/* v4-mapped addresses need IP_TOS */
-			setsockopt(conn->handle.fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos));
+			DISGUISE(setsockopt(conn->handle.fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos)));
 		else
-			setsockopt(conn->handle.fd, IPPROTO_IPV6, IPV6_TCLASS, &tos, sizeof(tos));
+			DISGUISE(setsockopt(conn->handle.fd, IPPROTO_IPV6, IPV6_TCLASS, &tos, sizeof(tos)));
 	}
 #endif
 }
@@ -604,7 +604,7 @@ static inline void conn_set_mark(const struct connection *conn, int mark)
 		return;
 
 #ifdef SO_MARK
-	setsockopt(conn->handle.fd, SOL_SOCKET, SO_MARK, &mark, sizeof(mark));
+	DISGUISE(setsockopt(conn->handle.fd, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)));
 #endif
 }
 
@@ -617,7 +617,7 @@ static inline void conn_set_quickack(const struct connection *conn, int value)
 		return;
 
 #ifdef TCP_QUICKACK
-	setsockopt(conn->handle.fd, IPPROTO_TCP, TCP_QUICKACK, &value, sizeof(value));
+	DISGUISE(setsockopt(conn->handle.fd, IPPROTO_TCP, TCP_QUICKACK, &value, sizeof(value)));
 #endif
 }
 
diff --git a/src/cli.c b/src/cli.c
index c1964df72..87d585259 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1717,7 +1717,7 @@ static int _getsocks(char **args, char *payload, struct appctx *appctx, void *pr
 		ha_warning("Cannot make the unix socket blocking\n");
 		goto out;
 	}
-	setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv));
+	DISGUISE(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)));
 	iov.iov_base = &tot_fd_nb;
 	iov.iov_len = sizeof(tot_fd_nb);
 	if (!(strm_li(s)->bind_conf->level & ACCESS_FD_LISTENERS))
diff --git a/src/dns.c b/src/dns.c
index 6a8ab831c..81144d303 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -207,7 +207,7 @@ static int dns_connect_namesaver(struct dns_nameserver *ns)
 	}
 
 	/* Make the socket non blocking */
-	fcntl(fd, F_SETFL, O_NONBLOCK);
+	DISGUISE(fcntl(fd, F_SETFL, O_NONBLOCK));
 
 	/* Add the fd in the fd list and update its parameters */
 	dgram->t.sock.fd = fd;
diff --git a/src/fd.c b/src/fd.c
index bf2383e4d..d1a7dfbf0 100644
--- a/src/fd.c
+++ b/src/fd.c
@@ -636,7 +636,7 @@ static int init_pollers_per_thread()
 
 	poller_rd_pipe = mypipe[0];
 	poller_wr_pipe[tid] = mypipe[1];
-	fcntl(poller_rd_pipe, F_SETFL, O_NONBLOCK);
+	DISGUISE(fcntl(poller_rd_pipe, F_SETFL, O_NONBLOCK));
 	fd_insert(poller_rd_pipe, poller_pipe_io_handler, poller_pipe_io_handler,
 	tid_bit);
 	fd_want_recv(poller_rd_pipe);
diff --git a/src/haproxy.c b/src/haproxy.c
index c3bc1f0e5..906f69903 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1160,7 +1160,7 @@ static int get_old_sockets(const char *unixsocket)
 			   unixsocket);
 		goto out;
 	}
-	setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv));
+	DISGUISE(setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv)));
 	iov.iov_base = &fd_nb;
 	iov.iov_len = sizeof(fd_nb);
 	msghdr.msg_iov = &iov;
diff --git a/src/listener.c b/src/listener.c
index cad0e0cc8..cadb99c59 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -802,7 +802,7 @@ void listener_accept(int fd)
 			(accept4_broken = 1
 #endif
 			if ((cfd = accept(fd, (struct sockaddr *)&addr, &laddr)) != -1)
-fcntl(cfd, F_SETFL, O_NONBLOCK);
+DISGUISE(fcntl(cfd, F_SETFL, O_NONBLOCK));
 
 		if (unlikely(cfd == -1)) {
 			switch (errno) {
diff --git a/src/log.c b/src/log.c
index 495a672d2..50d2ae4ec 100644
--- a/src/log.c
+++ b/src/log.c

haproxy@formilux.org

[Jerome Magnin]

Hi,

as I was trying to reproduce the issue with DNS Service Discovery with
SRV records reported in issue #775 I encountered a different issue. 

I am using bind as a dns server, and its answers contain an Authority
field before the Additional records that can be made use of since
13a9232eb ("MEDIUM: dns: use Additional records from SRV responses").
I found out that haproxy doesn't like that and would just consider the
whole response invalid, no address being set for my servers.

I've attached a patch to test for the type and just skip to the next
record, and it seems to work. I'm not sure it's the proper fix, as we
might want to test for the presence of an Authority field.

Note that the same config "works" before that commit. I understand
haproxy makes a lot more DNS requests before that commit, but what I
mean is that users migrating from earlier versions to 2.2 can have their
service break because of this.
-- 
Jérôme
>From 9637655e5ee0d4d51056cbdb948f4c2b1da272e4 Mon Sep 17 00:00:00 2001
From: Jerome Magnin 
Date: Sun, 26 Jul 2020 12:13:12 +0200
Subject: [PATCH] BUG/MAJOR: dns: don't treat Authority records as an error

Support for DNS Service Discovery by means of SRV records was enhanced with
commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
to use the content of the answers Additional records when present.

If there are Authority records before the Additional records we mistakenly
treat that as an invalid response. To fix this, we can skip to the next
record when the type is DNS_RTYPE_NS for the current record.
---
 include/haproxy/dns-t.h | 1 +
 src/dns.c   | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h
index b1b43976c..1a8e6e49f 100644
--- a/include/haproxy/dns-t.h
+++ b/include/haproxy/dns-t.h
@@ -71,6 +71,7 @@ extern struct pool_head *dns_requester_pool;
 
 /* dns record types (non exhaustive list) */
 #define DNS_RTYPE_A 1   /* IPv4 address */
+#define DNS_RTYPE_NS2   /* NS record type */
 #define DNS_RTYPE_CNAME 5   /* canonical name */
 #define DNS_RTYPE_  28  /* IPv6 address */
 #define DNS_RTYPE_SRV   33  /* SRV record */
diff --git a/src/dns.c b/src/dns.c
index 6a8ab831c..b840bf9de 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1076,6 +1076,12 @@ static int dns_validate_dns_response(unsigned char 
*resp, unsigned char *bufend,
dns_answer_record->type = reader[0] * 256 + reader[1];
reader += 2;
 
+   /* skip if record type is NS */
+   if (dns_answer_record->type == DNS_RTYPE_NS) {
+pool_free(dns_answer_item_pool, dns_answer_record);
+dns_answer_record = NULL;
+   continue;
+   }
/* 2 bytes for class (2) */
if (reader + 2 > bufend)
goto invalid_resp;
-- 
2.27.0

SRV records resolution failure if Authority section is present

[Jerome Magnin]

On Sun, Jul 26, 2020 at 01:21:45PM +0200, Jerome Magnin wrote:
> as I was trying to reproduce the issue with DNS Service Discovery with
> SRV records reported in issue #775 I encountered a different issue. 
> 
> I am using bind as a dns server, and its answers contain an Authority
> field before the Additional records that can be made use of since
> 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses").
> I found out that haproxy doesn't like that and would just consider the
> whole response invalid, no address being set for my servers.
> 
> I've attached a patch to test for the type and just skip to the next
> record, and it seems to work. I'm not sure it's the proper fix, as we
> might want to test for the presence of an Authority field.
> 
> Note that the same config "works" before that commit. I understand
> haproxy makes a lot more DNS requests before that commit, but what I
> mean is that users migrating from earlier versions to 2.2 can have their
> service break because of this.

Please find a proper fix attached. We already know if we have entries in
the Authority section (dns_p->header.nscount > 0), so just skip them
when they are present and only use the Additional records.

Jérôme
>From f77c279a8a3d40629d00238155b7adf941f1ccb6 Mon Sep 17 00:00:00 2001
From: Jerome Magnin 
Date: Sun, 26 Jul 2020 12:13:12 +0200
Subject: [PATCH] BUG/MAJOR: dns: don't treat Authority records as an error

Support for DNS Service Discovery by means of SRV records was enhanced with
commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
to use the content of the answers Additional records when present.

If there are Authority records before the Additional records we mistakenly
treat that as an invalid response. To fix this, just ignore the Authority
section if it exist and skip to the Additional records.
---
 src/dns.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/dns.c b/src/dns.c
index 6a8ab831c..292739b2a 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1044,6 +1044,31 @@ static int dns_validate_dns_response(unsigned char 
*resp, unsigned char *bufend,
if (dns_query->type != DNS_RTYPE_SRV)
goto skip_parsing_additional_records;
 
+   for (i = 0; i < dns_p->header.nscount; i++) {
+   offset = 0;
+   len = dns_read_name(resp, bufend, reader, tmpname, 
DNS_MAX_NAME_SIZE,
+   &offset, 0);
+   if (len == 0) {
+   continue;
+   }
+
+   if (reader + offset + 10 >= bufend)
+   goto invalid_resp;
+
+   reader += offset;
+   /* skip 2 bytes for class */
+   reader += 2;
+   /* skip 2 bytes for type */
+   reader += 2;
+   /* skip 2 bytes for ttl */
+   reader += 4;
+   /* read data len */
+   len = reader[0] * 256 + reader[1];
+   reader += 2;
+   if (reader + len >= bufend)
+   goto invalid_resp;
+   reader += len;
+   }
nb_saved_records = 0;
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
-- 
2.27.0

Re: SRV records resolution failure if Authority section is present

[Tim Düsterhus]

Jerome,

Am 26.07.20 um 17:25 schrieb Jerome Magnin:
> Please find a proper fix attached. We already know if we have entries in
> the Authority section (dns_p->header.nscount > 0), so just skip them
> when they are present and only use the Additional records.

Regarding the commit message: Please add backporting information to the
end of the commit message body (I believe it should be 2.2+).

Regarding the patch:

+   /* skip 2 bytes for ttl */
+   reader += 4;

Either the commit or the code is incorrect here.

Best regards
Tim Düsterhus

Re: DOC: Update documentation / comments to be gender neutral

[Jackie Tapia]

Thank you for the feedback! I'll make those changes.

On Thu, Jul 23, 2020 at 2:14 AM Tim Düsterhus  wrote:

> Jackie,
>
> First: I'm a community contributor, so my review might not necessarily
> reflect that of the HAProxy project. I'm also not a native English speaker.
>
> Regarding your patch I have a few comments:
>
> From your PR message:
>
> > Also, remove some trailing whitespaces from the files modified for
> funsies.
>
> I disagree with this change. This makes the patch larger than it needs
> to be and it distracts from the important parts of it. If anything it
> should become a dedicated patch that is separately committed.
>
> Regarding your commit messages:
>
> Please have a look at the CONTRIBUTING file you modified. Commit
> messages need to be prefixed with a "tag" ("DOC:" would probably be
> appropriate) and also contain a body:
>
> >As a rule of thumb, your patch MUST NEVER be made only of a subject
> line,
> >it *must* contain a description. Even one or two lines, or indicating
> >whether a backport is desired or not. It turns out that single-line
> commits
> >are so rare in the Git world that they require special manual (hence
> >painful) handling when they are backported, and at least for this
> reason
> >it's important to keep this in mind.
>
> (
> https://github.com/haproxy/haproxy/blob/f1ea47d8960730c79cc71fc634b3d7e5909d5683/CONTRIBUTING#L562-L567
> )
>
> Then to the patch itself:
>
> - It must not modify the license files doc/lgpl.txt and doc/gpl.txt,
> because they are standard license texts:
> https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html
> - In lua.txt it now reads "They just works carefully". I believe it
> should be "They just work carefully".
> - In proxy-protocol.txt it now reads "to hide it's activities". I
> believe it should be "to hide its activities".
>
> Best regards
> Tim Düsterhus
>


-- 

Jackie Tapia

Platform Software Engineer

Sprout Social

sproutsocial.com



Pronouns
:
She/Her/Hers

Re: SRV records resolution failure if Authority section is present

[Jerome Magnin]

Hi Tim,

On Sun, Jul 26, 2020 at 05:47:00PM +0200, Tim Düsterhus wrote:
> Jerome,
> 
> Regarding the commit message: Please add backporting information to the
> end of the commit message body (I believe it should be 2.2+).

You're right the commit I mentionned in the message was indeed
introduced in 2.2-dev, so this must be backported to 2.2.
> 
> Regarding the patch:
> 
> + /* skip 2 bytes for ttl */
> + reader += 4;
> 
> Either the commit or the code is incorrect here.
The comment was wrong indeed. Thanks for your review.

-- 
Jérôme
>From 363ed1dd2f3ded7837bbb424eabb309803fc6292 Mon Sep 17 00:00:00 2001
From: Jerome Magnin 
Date: Sun, 26 Jul 2020 12:13:12 +0200
Subject: [PATCH] BUG/MAJOR: dns: don't treat Authority records as an error

Support for DNS Service Discovery by means of SRV records was enhanced with
commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
to use the content of the answers Additional records when present.

If there are Authority records before the Additional records we mistakenly
treat that as an invalid response. To fix this, just ignore the Authority
section if it exist and skip to the Additional records.

As 13a9232eb was introduced during 2.2-dev, it must be backported to 2.2.
---
 src/dns.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/src/dns.c b/src/dns.c
index 6a8ab831c..7ea35ac11 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1044,6 +1044,33 @@ static int dns_validate_dns_response(unsigned char 
*resp, unsigned char *bufend,
if (dns_query->type != DNS_RTYPE_SRV)
goto skip_parsing_additional_records;
 
+   for (i = 0; i < dns_p->header.nscount; i++) {
+   offset = 0;
+   len = dns_read_name(resp, bufend, reader, tmpname, 
DNS_MAX_NAME_SIZE,
+   &offset, 0);
+   if (len == 0) {
+   continue;
+   }
+
+   if (reader + offset + 10 >= bufend)
+   goto invalid_resp;
+
+   reader += offset;
+   /* skip 2 bytes for class */
+   reader += 2;
+   /* skip 2 bytes for type */
+   reader += 2;
+   /* skip 4 bytes for ttl */
+   reader += 4;
+   /* read data len */
+   len = reader[0] * 256 + reader[1];
+   reader += 2;
+
+   if (reader + len >= bufend)
+   goto invalid_resp;
+
+   reader += len;
+   }
nb_saved_records = 0;
for (i = 0; i < dns_p->header.arcount; i++) {
if (reader >= bufend)
-- 
2.27.0

Re: DOC: Update documentation / comments to be gender neutral

[Jackie Tapia]

I've attached an updated patch.

Thanks!



On Sun, Jul 26, 2020 at 10:58 AM Jackie Tapia 
wrote:

> Thank you for the feedback! I'll make those changes.
>
> On Thu, Jul 23, 2020 at 2:14 AM Tim Düsterhus  wrote:
>
>> Jackie,
>>
>> First: I'm a community contributor, so my review might not necessarily
>> reflect that of the HAProxy project. I'm also not a native English
>> speaker.
>>
>> Regarding your patch I have a few comments:
>>
>> From your PR message:
>>
>> > Also, remove some trailing whitespaces from the files modified for
>> funsies.
>>
>> I disagree with this change. This makes the patch larger than it needs
>> to be and it distracts from the important parts of it. If anything it
>> should become a dedicated patch that is separately committed.
>>
>> Regarding your commit messages:
>>
>> Please have a look at the CONTRIBUTING file you modified. Commit
>> messages need to be prefixed with a "tag" ("DOC:" would probably be
>> appropriate) and also contain a body:
>>
>> >As a rule of thumb, your patch MUST NEVER be made only of a subject
>> line,
>> >it *must* contain a description. Even one or two lines, or indicating
>> >whether a backport is desired or not. It turns out that single-line
>> commits
>> >are so rare in the Git world that they require special manual (hence
>> >painful) handling when they are backported, and at least for this
>> reason
>> >it's important to keep this in mind.
>>
>> (
>> https://github.com/haproxy/haproxy/blob/f1ea47d8960730c79cc71fc634b3d7e5909d5683/CONTRIBUTING#L562-L567
>> )
>>
>> Then to the patch itself:
>>
>> - It must not modify the license files doc/lgpl.txt and doc/gpl.txt,
>> because they are standard license texts:
>> https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html
>> - In lua.txt it now reads "They just works carefully". I believe it
>> should be "They just work carefully".
>> - In proxy-protocol.txt it now reads "to hide it's activities". I
>> believe it should be "to hide its activities".
>>
>> Best regards
>> Tim Düsterhus
>>
>
>
> --
>
> Jackie Tapia
>
> Platform Software Engineer
>
> Sprout Social
>
> sproutsocial.com
>
>
>
> Pronouns
> :
> She/Her/Hers
>


-- 

Jackie Tapia

Platform Software Engineer

Sprout Social

sproutsocial.com



Pronouns
:
She/Her/Hers


0001-DOC-update-documentation-comments-to-be-gender-neutr.patch
Description: Binary data

Re: DOC: Update documentation / comments to be gender neutral

[Tim Düsterhus]

Jackie,

Am 26.07.20 um 18:36 schrieb Jackie Tapia:
> I've attached an updated patch.
> 
Thank you, the Diff LGTM now (however I did not check whether you missed
any gendered phrasing).

Unfortunately it appears that you missed by remark regarding the missing
commit message body.

Please make sure to always use a single line commit title *plus* at
least a single line commit message body. If the title wraps within the
generated patch files it is probably too long.

Based off your patch and existing commit message, may I suggest the
following?

---
DOC: Use gender neutral language

This patch updates the documentation files and code comments to avoid
the use of gender specific phrasing in favor of "they" or "it".
---

Best regards
Tim Düsterhus

Re: DOC: Update documentation / comments to be gender neutral

[Jackie Tapia]

Thanks for the suggestion, Tim.

That commit message sounds good to me.

On Sun, Jul 26, 2020 at 11:49 AM Tim Düsterhus  wrote:

> Jackie,
>
> Am 26.07.20 um 18:36 schrieb Jackie Tapia:
> > I've attached an updated patch.
> >
> Thank you, the Diff LGTM now (however I did not check whether you missed
> any gendered phrasing).
>
> Unfortunately it appears that you missed by remark regarding the missing
> commit message body.
>
> Please make sure to always use a single line commit title *plus* at
> least a single line commit message body. If the title wraps within the
> generated patch files it is probably too long.
>
> Based off your patch and existing commit message, may I suggest the
> following?
>
> ---
> DOC: Use gender neutral language
>
> This patch updates the documentation files and code comments to avoid
> the use of gender specific phrasing in favor of "they" or "it".
> ---
>
> Best regards
> Tim Düsterhus
>
>

-- 

Jackie Tapia

Platform Software Engineer

Sprout Social

sproutsocial.com



Pronouns
:
She/Her/Hers


0001-DOC-Use-gender-neutral-language.patch
Description: Binary data

Re: DOC: Update documentation / comments to be gender neutral

[Tim Düsterhus]

Jackie,

Am 26.07.20 um 19:02 schrieb Jackie Tapia:
> Thanks for the suggestion, Tim.
> 
> That commit message sounds good to me.
> 

Your patch looks good to me now. I've put Willy into Cc to perform the
final review and commit the patch.

Best regards
Tim Düsterhus

Re: DOC: Update documentation / comments to be gender neutral

[Willy Tarreau]

Hi,

On Sun, Jul 26, 2020 at 07:54:17PM +0200, Tim Düsterhus wrote:
> Jackie,
> 
> Am 26.07.20 um 19:02 schrieb Jackie Tapia:
> > Thanks for the suggestion, Tim.
> > 
> > That commit message sounds good to me.
> > 
> 
> Your patch looks good to me now. I've put Willy into Cc to perform the
> final review and commit the patch.

Thanks Jackie, and thanks for handling this review, Tim.

I've read it (and indeed it's much easier now that the first one) and
overall it looks good to me as well.

Just a small point, Jackie, as I noticed you fixed several bad constructs
of "he" instead of "it", it's worth noting that in some latin languages
(like French or Spanish), a number of common words are arbitrarily male
or female. A car is female, a truck is male, a plane is male and a space
shuttle is female. I could give plenty of examples like this, as there's
almost no equivalent for "it". It gets funny when (ab)using some english
words in our jobs because they automatically get the gender of their local
equivalent. As you noticed, some of these slip from French to English when
docs are written, beause while the exercise of translating what you think
to another language is hard, the exercise of thinking in another one is
even harder, and sometimes reading such things are even not noticed, they
are hard-wired in one's brain. As such I think that adopting an advanced
form of English that's not even taught in schools will be even harder to
follow and understand for most of us non-native users (hence the efforts
of "he/she" that you've noticed at a few places).

This comforts my beliefs that avoiding mentions of the user in general is
by far the best solution, and that when not possible (due to generalizing),
it's best to use a plural form (since in this case what is described doesn't
apply to a single user but to all of them). In this case instead of writing
"upon error on any of the parallel requests sent by a user, he/she will be
notified by an error message" (which admittedly looks ugly), or "upon error
on any of the parallel requests sent by a user, they will be notified by an
error message" (which is now wrong since it parses as the requests being
notified instead of the user, at least to any of us for whom English was
only taught as a second language), we'd rather write this using a repetitive
form like "... the user will be notified", or an alternative one like "Users
whose requests triggering an error will be notified by a response...". This
avoids such advanced and difficult forms that are clearly not mastered by
all those who almost only use english to write documentation.

Overall, while I'm used to say that "I/you/we" should absolutely be avoided
in docs (it's not the place to discuss with the user but to explain rules),
we could probably mention that "I/you/he/she/we" are to be avoided and that
the only pronouns left are "it" for singular and "they" for plural, implying
that users will always be designated using the plural form. This simplifies
everything and will force doc writers to think about simpler sentences that
are less ambiguous, and more importantly that are understandable even by
the many who don't have 10 years of English practice behind them.

Regards,
Willy

Re: SRV records resolution failure if Authority section is present

[Willy Tarreau]

Thanks Jérôme,

CCing Baptiste for approval (in case we've missed anything, I'm clueless
about DNS).

Willy

On Sun, Jul 26, 2020 at 06:04:38PM +0200, Jerome Magnin wrote:
> Hi Tim,
> 
> On Sun, Jul 26, 2020 at 05:47:00PM +0200, Tim Düsterhus wrote:
> > Jerome,
> > 
> > Regarding the commit message: Please add backporting information to the
> > end of the commit message body (I believe it should be 2.2+).
> 
> You're right the commit I mentionned in the message was indeed
> introduced in 2.2-dev, so this must be backported to 2.2.
> > 
> > Regarding the patch:
> > 
> > +   /* skip 2 bytes for ttl */
> > +   reader += 4;
> > 
> > Either the commit or the code is incorrect here.
> The comment was wrong indeed. Thanks for your review.
> 
> -- 
> Jérôme

> >From 363ed1dd2f3ded7837bbb424eabb309803fc6292 Mon Sep 17 00:00:00 2001
> From: Jerome Magnin 
> Date: Sun, 26 Jul 2020 12:13:12 +0200
> Subject: [PATCH] BUG/MAJOR: dns: don't treat Authority records as an error
> 
> Support for DNS Service Discovery by means of SRV records was enhanced with
> commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
> to use the content of the answers Additional records when present.
> 
> If there are Authority records before the Additional records we mistakenly
> treat that as an invalid response. To fix this, just ignore the Authority
> section if it exist and skip to the Additional records.
> 
> As 13a9232eb was introduced during 2.2-dev, it must be backported to 2.2.
> ---
>  src/dns.c | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/src/dns.c b/src/dns.c
> index 6a8ab831c..7ea35ac11 100644
> --- a/src/dns.c
> +++ b/src/dns.c
> @@ -1044,6 +1044,33 @@ static int dns_validate_dns_response(unsigned char 
> *resp, unsigned char *bufend,
>   if (dns_query->type != DNS_RTYPE_SRV)
>   goto skip_parsing_additional_records;
>  
> + for (i = 0; i < dns_p->header.nscount; i++) {
> + offset = 0;
> + len = dns_read_name(resp, bufend, reader, tmpname, 
> DNS_MAX_NAME_SIZE,
> + &offset, 0);
> + if (len == 0) {
> + continue;
> + }
> +
> + if (reader + offset + 10 >= bufend)
> + goto invalid_resp;
> +
> + reader += offset;
> + /* skip 2 bytes for class */
> + reader += 2;
> + /* skip 2 bytes for type */
> + reader += 2;
> + /* skip 4 bytes for ttl */
> + reader += 4;
> + /* read data len */
> + len = reader[0] * 256 + reader[1];
> + reader += 2;
> +
> + if (reader + len >= bufend)
> + goto invalid_resp;
> +
> + reader += len;
> + }
>   nb_saved_records = 0;
>   for (i = 0; i < dns_p->header.arcount; i++) {
>   if (reader >= bufend)
> -- 
> 2.27.0
> 

Re: DOC: Update documentation / comments to be gender neutral

[Tim Düsterhus]

Willy,

Am 26.07.20 um 22:34 schrieb Willy Tarreau:
> Just a small point, Jackie, as I noticed you fixed several bad constructs
> of "he" instead of "it", it's worth noting that in some latin languages
> (like French or Spanish), a number of common words are arbitrarily male
> or female. A car is female, a truck is male, a plane is male and a space
> shuttle is female. I could give plenty of examples like this, as there's
> almost no equivalent for "it". [...]

Today I learned. I thought that having gendered nouns was unique to
German or as Mark Twain said in "The Awful German Language"
(https://en.wikipedia.org/wiki/The_Awful_German_Language):

"Every noun has a gender, and there is no sense or system in
distribution; so the gender of each must be learned separately and by
heart. There is no other way. To do this one has to have a memory like a
memorandum-book. In German, a young lady has no sex, while a turnip has.
Think what overwrought reverence that shows for the turnip, and what
callous disrespect for the girl."

And indeed sometimes I accidentally slip in a gendered pronoun when
communicating in English.

> This comforts my beliefs that avoiding mentions of the user in general is
> by far the best solution, and that when not possible (due to generalizing),
> it's best to use a plural form (since in this case what is described doesn't
> apply to a single user but to all of them). In this case instead of writing

Please note that the "they" is no plural form here. It's a so-called
"Singular they": https://en.wikipedia.org/wiki/Singular_they

In German we don't have an equivalent for that. I don't know about
French. By now it feels pretty natural to me, though.

Best regards
Tim Düsterhus

Re: DOC: Update documentation / comments to be gender neutral

[Willy Tarreau]

On Mon, Jul 27, 2020 at 12:10:02AM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 26.07.20 um 22:34 schrieb Willy Tarreau:
> > Just a small point, Jackie, as I noticed you fixed several bad constructs
> > of "he" instead of "it", it's worth noting that in some latin languages
> > (like French or Spanish), a number of common words are arbitrarily male
> > or female. A car is female, a truck is male, a plane is male and a space
> > shuttle is female. I could give plenty of examples like this, as there's
> > almost no equivalent for "it". [...]
> 
> Today I learned. I thought that having gendered nouns was unique to
> German or as Mark Twain said in "The Awful German Language"
> (https://en.wikipedia.org/wiki/The_Awful_German_Language):
(...)

Then I learned as well :-)  Maybe in the end it's only English that has
no gender! English people speaking french are often having difficulties
spotting the right gender, which makes their sentences sound funny but
this doesn't affect comprehension at all since the genders for things do
not come from anything logical.

> > This comforts my beliefs that avoiding mentions of the user in general is
> > by far the best solution, and that when not possible (due to generalizing),
> > it's best to use a plural form (since in this case what is described doesn't
> > apply to a single user but to all of them). In this case instead of writing
> 
> Please note that the "they" is no plural form here. It's a so-called
> "Singular they": https://en.wikipedia.org/wiki/Singular_they

I noticed, but it's definitely not the level of detail that we were
taught in English courses at school. When you have 1-2 hours a week,
there are definitely more important things to put the focus on, and
most of what many of us learned was through participation on mailing
lists.

> In German we don't have an equivalent for that. I don't know about
> French. By now it feels pretty natural to me, though.

We don't have an equivalent either, hence the use of "he" for "it"
sometimes, or "he/she" for a more polite or inclusive form, though
for to designate a person. For "his/her/their" we use the same term
("leur") so that removes the need to think about it at all.

But when you look at the examples in the link above, "They are my child",
I'm pretty sure I would have had a zero if I gave a copy with that to any
of my teachers. That's why I'm saying that using such advanced forms should
definitely be frowned upon especially when the user has so little place in
a project like haproxy where both sides are just HTTP agents, and that for
the rare cases where this would be needed, we could easily generalize to
"users" and avoid any confusion, and not require anyone who barely expresses
themself with 400 words to require such difficult rules. That's only my
point: being inclusive, but to the non-native speakers as well.

Cheers,
Willy