Re: [PATCH] Minor improvements to doc "http-request set-src"
On Tue, Apr 21, 2020 at 04:36:55PM +0200, Tim Düsterhus wrote: > Olivier, > > Am 21.04.20 um 16:34 schrieb Olivier D: > > ;) > > Patch updated attached. > > > > Now LGTM. > > Reviewed-by: Tim Duesterhus Thanks guys, now applied. Olivier, I noticed something strange, your patch was produced without the usual a/ b/ prefixes and the file started at doc/. It looks as if you had produced the patches using "--no-prefix", which then fails to apply, so it's worth having a look at your setup. I didn't have problems applying it with "patch -p0<" though, so the rest was OK. Thanks, Willy
Re: [PATCH] Minor improvements to doc "http-request set-src"
On Tue, Apr 21, 2020 at 12:56:51PM +0200, Tim Düsterhus wrote: > PS: Personal opinion, but I prefer quotes in replies to be shortened as > much as possible, while still providing context. I don't want to scroll > through kilobytes of stuff I've already seen :-) Rest assured it's a shared opinion, as I also hate having to scroll far away, and sometimes even miss isolated responses! :-) Willy
Re: [PATCH] Minor improvements to doc "http-request set-src"
Hi,
Le mar. 21 avr. 2020 à 12:56, Tim Düsterhus a écrit :
> Olivier,
>
> PS: Personal opinion, but I prefer quotes in replies to be shortened as
> much as possible, while still providing context. I don't want to scroll
> through kilobytes of stuff I've already seen :-)
>
;)
Patch updated attached.
From e6b11f3a795ec40c8b802d9d1190f3f6bbd15f5d Mon Sep 17 00:00:00 2001
From: Olivier Doucet
Date: Tue, 21 Apr 2020 09:32:56 +0200
Subject: [PATCH] DOC: Improve documentation on http-request set-src
This patch adds more explanation on how to use "http-request set-src"
and a link to "option forwardfor".
This patch can be applied to all previous version starting at 1.6
---
doc/configuration.txt | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git doc/configuration.txt doc/configuration.txt
index 5d01835d7..e695ab7f5 100644
--- doc/configuration.txt
+++ doc/configuration.txt
@@ -5114,16 +5114,23 @@ http-request set-src [ { if | unless }
]
This is used to set the source IP address to the value of specified
expression. Useful when a proxy in front of HAProxy rewrites source IP, but
provides the correct IP in a HTTP header; or you want to mask source IP for
- privacy.
+ privacy. All subsequent calls to "src" fetch will return this value
+ (see example).
Arguments :
Is a standard HAProxy expression formed by a sample-fetch followed
by some converters.
+ See also "option forwardfor".
+
Example:
http-request set-src hdr(x-forwarded-for)
http-request set-src src,ipmask(24)
+# After the masking this will track connections
+# based on the IP address with the last byte zeroed out.
+http-request track-sc0 src
+
When possible, set-src preserves the original source port as long as the
address family allows it, otherwise the source port is set to 0.
--
2.18.0.windows.1
Re: [PATCH] Minor improvements to doc "http-request set-src"
Olivier, Am 21.04.20 um 09:37 schrieb Olivier D: > Thank you for your valuable feedback. Find attached a new patch will all > your comments taken into account. > I've missed two more little things during my initial review: 1. The Subject of the patch should start with "DOC:" instead of "[DOC]". 2. All subsequent calls to src field will return this value (see example). -> It's not "field", but "fetch". Not sure whether "src" should also be quoted in there. Other than that it looks good to me now. Best regards Tim Düsterhus PS: Personal opinion, but I prefer quotes in replies to be shortened as much as possible, while still providing context. I don't want to scroll through kilobytes of stuff I've already seen :-)
Re: [PATCH] Minor improvements to doc "http-request set-src"
Hello,
Le lun. 20 avr. 2020 à 20:37, Tim Düsterhus a écrit :
> Olivier,
>
> Am 20.04.20 um 20:03 schrieb Olivier D:
> > I'm using gmail so I add to attach patches and was not able to send them
> > directly. If format is wrong, tell me :)
> >
>
> Format looks good to me. Your commit message however does not (fully)
> follow the instructions within the CONTRIBUTING file
> (
> https://github.com/haproxy/haproxy/blob/dfad6a41ad9f012671b703788dd679cf24eb8c5a/CONTRIBUTING#L562-L567
> ):
>
> >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.
>
> Regarding the patch itself:
>
> > diff --git doc/configuration.txt doc/configuration.txt
> > index 5d01835d7..ddfabcd92 100644
> > --- doc/configuration.txt
> > +++ doc/configuration.txt
> > @@ -6735,7 +6735,8 @@ option forwardfor [ except ] [ header
> ] [ if-none ]
> >header for a known source address or network by adding the "except"
> keyword
> >followed by the network address. In this case, any source IP matching
> the
> >network will not cause an addition of this header. Most common uses
> are with
> > - private networks or 127.0.0.1.
> > + private networks or 127.0.0.1. Another way to do it is to tell
> HAProxy to
> > + trust a custom header with "http-request set-src".
>
> This change looks incorrect to me. "option forwardfor" is for sending,
> not "receiving" IP addresses.
>
> >Alternatively, the keyword "if-none" states that the header will only
> be
> >added if it is not present. This should only be used in perfectly
> trusted
> > @@ -6760,6 +6761,14 @@ option forwardfor [ except ] [ header
> ] [ if-none ]
> > mode http
> > option forwardfor header X-Client
> >
> > + Example :
> > +# Trust a specific header and use it as origin IP.
> > +# If not found, source IP will be used.
> > +frontend www
> > +mode http
> > +http-request set-src CF-Connecting-IP
>
> I believe this should read `http-request set-src
> %[req.hdr(CF-Connecting-IP)]`. However:
>
> 1. I don't like having company specific headers in there. Especially
> since Cloudflare supports the standard XFF.
> 2. I don't consider that a useful addition.
>
> > +option forwardfor
> > +
> >See also : "option httpclose", "option http-server-close",
> > "option http-keep-alive"
> >
>
> Patch 2:
>
> > diff --git doc/configuration.txt doc/configuration.txt
> > index ddfabcd92..49324fa53 100644
> > --- doc/configuration.txt
> > +++ doc/configuration.txt
> > @@ -5114,7 +5114,8 @@ http-request set-src [ { if | unless }
> ]
> >This is used to set the source IP address to the value of specified
> >expression. Useful when a proxy in front of HAProxy rewrites source
> IP, but
> >provides the correct IP in a HTTP header; or you want to mask source
> IP for
> > - privacy.
> > + privacy. All subsequent calls to src field will return this value
> > + (see example).
>
> This change looks good to me.
>
> >Arguments :
> >Is a standard HAProxy expression formed by a sample-fetch
> followed
> > @@ -5124,6 +5125,11 @@ http-request set-src [ { if | unless }
> ]
> > http-request set-src hdr(x-forwarded-for)
> > http-request set-src src,ipmask(24)
> >
> > + Example:
>
> Only a single "Example:" heading is used throughout the documentation.
> As the first line can be shared with the previous example you could
> write something like: # After the masking this will track connections
> based on the IP address with the last octet zeroed out.
>
> > +# This will track connection based on header IP
> > +http-request set-src hdr(x-forwarded-for)
> > +http-request track-sc0 src
> > +
> >When possible, set-src preserves the original source port as long as
> the
> >address family allows it, otherwise the source port is set to 0.
>
Thank you for your valuable feedback. Find attached a new patch will all
your comments taken into account.
Olivier
>
> Best regards
> Tim Düsterhus
>
From e6b11f3a795ec40c8b802d9d1190f3f6bbd15f5d Mon Sep 17 00:00:00 2001
From: Olivier Doucet
Date: Tue, 21 Apr 2020 09:32:56 +0200
Subject: [PATCH] [DOC] Improve documentation on http-request set-src
This patch adds more explanation on how to use "http-request set-src"
and a link to "option forwardfor".
This patch can be applied to all previous version starting at 1.6
---
doc/configuration.txt | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git doc/configuration.txt doc/configuration.txt
index 5d01835d7..e695ab7f5 100644
--- doc/configuration.txt
+++ doc/configuration.txt
@@ -5114,16 +5114,23 @@
Re: [PATCH] Minor improvements to doc "http-request set-src"
Olivier,
Am 20.04.20 um 20:03 schrieb Olivier D:
> I'm using gmail so I add to attach patches and was not able to send them
> directly. If format is wrong, tell me :)
>
Format looks good to me. Your commit message however does not (fully)
follow the instructions within the CONTRIBUTING file
(https://github.com/haproxy/haproxy/blob/dfad6a41ad9f012671b703788dd679cf24eb8c5a/CONTRIBUTING#L562-L567):
>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.
Regarding the patch itself:
> diff --git doc/configuration.txt doc/configuration.txt
> index 5d01835d7..ddfabcd92 100644
> --- doc/configuration.txt
> +++ doc/configuration.txt
> @@ -6735,7 +6735,8 @@ option forwardfor [ except ] [ header
> ] [ if-none ]
>header for a known source address or network by adding the "except" keyword
>followed by the network address. In this case, any source IP matching the
>network will not cause an addition of this header. Most common uses are
> with
> - private networks or 127.0.0.1.
> + private networks or 127.0.0.1. Another way to do it is to tell HAProxy to
> + trust a custom header with "http-request set-src".
This change looks incorrect to me. "option forwardfor" is for sending,
not "receiving" IP addresses.
>Alternatively, the keyword "if-none" states that the header will only be
>added if it is not present. This should only be used in perfectly trusted
> @@ -6760,6 +6761,14 @@ option forwardfor [ except ] [ header
> ] [ if-none ]
> mode http
> option forwardfor header X-Client
>
> + Example :
> +# Trust a specific header and use it as origin IP.
> +# If not found, source IP will be used.
> +frontend www
> +mode http
> +http-request set-src CF-Connecting-IP
I believe this should read `http-request set-src
%[req.hdr(CF-Connecting-IP)]`. However:
1. I don't like having company specific headers in there. Especially
since Cloudflare supports the standard XFF.
2. I don't consider that a useful addition.
> +option forwardfor
> +
>See also : "option httpclose", "option http-server-close",
> "option http-keep-alive"
>
Patch 2:
> diff --git doc/configuration.txt doc/configuration.txt
> index ddfabcd92..49324fa53 100644
> --- doc/configuration.txt
> +++ doc/configuration.txt
> @@ -5114,7 +5114,8 @@ http-request set-src [ { if | unless }
> ]
>This is used to set the source IP address to the value of specified
>expression. Useful when a proxy in front of HAProxy rewrites source IP, but
>provides the correct IP in a HTTP header; or you want to mask source IP for
> - privacy.
> + privacy. All subsequent calls to src field will return this value
> + (see example).
This change looks good to me.
>Arguments :
>Is a standard HAProxy expression formed by a sample-fetch
> followed
> @@ -5124,6 +5125,11 @@ http-request set-src [ { if | unless }
> ]
> http-request set-src hdr(x-forwarded-for)
> http-request set-src src,ipmask(24)
>
> + Example:
Only a single "Example:" heading is used throughout the documentation.
As the first line can be shared with the previous example you could
write something like: # After the masking this will track connections
based on the IP address with the last octet zeroed out.
> +# This will track connection based on header IP
> +http-request set-src hdr(x-forwarded-for)
> +http-request track-sc0 src
> +
>When possible, set-src preserves the original source port as long as the
>address family allows it, otherwise the source port is set to 0.
Best regards
Tim Düsterhus
[PATCH] Minor improvements to doc "http-request set-src"
Hello,
Find attached two small patches to improve documentation on "option
forwardfor" and "http-request set-src".
I'm using gmail so I add to attach patches and was not able to send them
directly. If format is wrong, tell me :)
Olivier
From efbc320861c9c5a43219983cfc1073070b3e6622 Mon Sep 17 00:00:00 2001
From: Olivier Doucet
Date: Mon, 20 Apr 2020 19:39:27 +0200
Subject: [DOC] This patch adds example on how to use "http-request
set-src" with "option forwardfor".
---
doc/configuration.txt | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git doc/configuration.txt doc/configuration.txt
index 5d01835d7..ddfabcd92 100644
--- doc/configuration.txt
+++ doc/configuration.txt
@@ -6735,7 +6735,8 @@ option forwardfor [ except ] [ header ]
[ if-none ]
header for a known source address or network by adding the "except" keyword
followed by the network address. In this case, any source IP matching the
network will not cause an addition of this header. Most common uses are with
- private networks or 127.0.0.1.
+ private networks or 127.0.0.1. Another way to do it is to tell HAProxy to
+ trust a custom header with "http-request set-src".
Alternatively, the keyword "if-none" states that the header will only be
added if it is not present. This should only be used in perfectly trusted
@@ -6760,6 +6761,14 @@ option forwardfor [ except ] [ header ]
[ if-none ]
mode http
option forwardfor header X-Client
+ Example :
+# Trust a specific header and use it as origin IP.
+# If not found, source IP will be used.
+frontend www
+mode http
+http-request set-src CF-Connecting-IP
+option forwardfor
+
See also : "option httpclose", "option http-server-close",
"option http-keep-alive"
--
2.18.0.windows.1
From 34efa737cf09753301787dde7dc77df2041b3288 Mon Sep 17 00:00:00 2001
From: Olivier Doucet
Date: Mon, 20 Apr 2020 19:59:43 +0200
Subject: [DOC] add useful informations on "http-request set-src"
---
doc/configuration.txt | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git doc/configuration.txt doc/configuration.txt
index ddfabcd92..49324fa53 100644
--- doc/configuration.txt
+++ doc/configuration.txt
@@ -5114,7 +5114,8 @@ http-request set-src [ { if | unless }
]
This is used to set the source IP address to the value of specified
expression. Useful when a proxy in front of HAProxy rewrites source IP, but
provides the correct IP in a HTTP header; or you want to mask source IP for
- privacy.
+ privacy. All subsequent calls to src field will return this value
+ (see example).
Arguments :
Is a standard HAProxy expression formed by a sample-fetch followed
@@ -5124,6 +5125,11 @@ http-request set-src [ { if | unless }
]
http-request set-src hdr(x-forwarded-for)
http-request set-src src,ipmask(24)
+ Example:
+# This will track connection based on header IP
+http-request set-src hdr(x-forwarded-for)
+http-request track-sc0 src
+
When possible, set-src preserves the original source port as long as the
address family allows it, otherwise the source port is set to 0.
--
2.18.0.windows.1
