Re: sticky session with cookie
~. for strictness -- Ionel GARDAIS Tech'Advantage CIO - IT Team manager De: "Ionel GARDAIS" À: "haproxy" Envoyé: Vendredi 24 Janvier 2020 19:24:45 Objet: sticky session with cookie Hi list, I'm sorry if this question has already been asked but I don't see any answer to it. The backend servers already set a cookie which value identifies it (.) Can haproxy archieve stickiness based on the value ? Currently, I'm using cookie AUTH_SESSION_ID prefix server srv1 192.168.1.1 cookie s1 server srv2 192.168.1.2 cookie s2 but ending with .. is redundant if I can make haproxy use Thanks, Ionel -- 232 avenue Napoleon BONAPARTE 92500 RUEIL MALMAISON Capital EUR 219 300,00 - RCS Nanterre B 408 832 301 - TVA FR 09 408 832 301
sticky session with cookie
Hi list, I'm sorry if this question has already been asked but I don't see any answer to it. The backend servers already set a cookie which value identifies it (.) Can haproxy archieve stickiness based on the value ? Currently, I'm using cookie AUTH_SESSION_ID prefix server srv1 192.168.1.1 cookie s1 server srv2 192.168.1.2 cookie s2 but ending with .. is redundant if I can make haproxy use Thanks, Ionel -- 232 avenue Napoleon BONAPARTE 92500 RUEIL MALMAISON Capital EUR 219 300,00 - RCS Nanterre B 408 832 301 - TVA FR 09 408 832 301
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
> Le 24 janv. 2020 à 16:38, William Lallemand a écrit : > > On Fri, Jan 24, 2020 at 01:22:05PM +0100, Emmanuel Hocdet wrote: >> >> Hi William, >> > Hello Manu! > >>> Le 23 janv. 2020 à 16:20, William Lallemand a >>> écrit : >>> >>> That's not a good idea to be able to add a new path to the list each time >>> this >>> keyword is found, this is not how the configuration works in the global >>> section, a new line with the keyword should remove the previous one. >>> >> >> Okay, or refuse the second one. > > That's really important to be able to overwrite the previous value, because > the > global section could be superseded. For example in the ALOHA appliances, there > is a global.def file which is a configuration file with default values, and > these values can be overwritten in the haproxy.cfg. > >> To be clear, 'issuer-path' can be confused, i could be named >> 'issuers-chain-path’. >> > > That might be a good idea. > - PEM files into one (e.g. cat cert.pem key.pem > combined.pem). If your CA + PEM files into one (e.g. cat key.pem cert.pem > combined.pem). If your CA >>> >>> >>> I'm not sure to understand why this was changed, maybe by mistake? If not >>> could >>> you elaborate in the commit message, or split this part in another commit if >>> it's not related to the feature. >>> >> >> Indeed, is not related. It’s more optimal when PEM is parsed. >> Key is parsed first, Cert next after rewind the BIO at begining. >> > > Oh, that's right. We can probably update the documentation by explaining > why it's more efficient in this order. I might be wrong but I think the > previous order was mandatory because of the way the certificates were loaded > before, because it wasn't using the BIO, but the high level functions of > openssl. > >> >> It’s not really magic, it’s based on X509, how certificate are build, and >> and how to check(find) an issuer for a certificate (via X509_check_issued >> with openssl). >> It’s what libssl do to verify certificat, find the appropriate root CA >> (loaded via ca-file in >> HAproxy). > > I understand the principle, but my fear is the fact that its a global context > only referenced by IDs, and you don't specified which file are used in the > configuration. > >> For certificats generated via the same issuer, there are no reason to not >> complete it >> with the same chain. At worst, not used the functionality (include a chain >> who match the issuer). >> I have experienced this for years on our multi-tenant platform. Customers >> upload >> their certificates (the chain are ignored) and we accept it if we can verify >> it. >> For that we need, in addition to "ca-certificates » pkg, all intermediate >> certificates from >> accepted CA (Comodo, LetsEncrypt, CAcert, Cloudflare, …). >> When PEM file are generated for HAproxy, the chain of issuers is construct >> from >> all this intermediate certificates. This guarantees to have a correct chain >> for all certificates, >> and chain can be updated automatically. >> When intermediate certificates repository is updated (by humans) we can have >> root >> certificate (detected and ignored), or more than one issuer for a >> certificate (sha1/sha256 ...): >> the better (valid) issuer is chosen. >> (1) For haproxy, I didn't want to do that initially, because its’ tricky and >> too "magic ». Add such >> feature without change the initial behaviour: >> Have PEM with Key+Cert+Chain or have Key+Cert and complete the Chain (via >> the start of >> the chain - the issuer -). >> > > Ok I understand your usage better, it completely makes sense. So the provider > will fill itself the directory with trusted files, this is not something which > will be filled by the customer. > >>> Maybe we should use a keyword on the bind line to load it instead of this, >>> like >>> `ssl crt foo.pem issuer bar.pem` and this keyword could be applied to a lot >>> of certificates by using the default-server keyword. >>> >> (default-server -> ssl-default-bind-options) >> (‘issuer’ -> ‘issuers-chain’ to be less confused) >> I don’t like that very much. We can have a lot of certificats with different >> issuer, and only crt-list >> with this issuers-chain param could do the job. It’s very unwieldy. On the >> other hand it takes >> away any discovery, It might be more in line with how HAproxy deal with ssl >> files. > > That's my concern, but with your explanations I understand that the usage is > not really compatible with the way haproxy deals with ssl files. But we should > handle it the same way as we plan to handle the certificate directories. > The real usage is only share issuers-chain in memory and greatly speedup reload :) The ‘magic’ is done in batch process. I think line like ‘ssl crt foo.pem chain bar.pem’ could do the job. >> It could looks like ca-file, but ca-file is a PEM who can contient all >> CA-root. >> A issuers-file could have all issuers in a raw, but we need to
Re: Disabling regtests in Travis ?
On Fri, Jan 24, 2020 at 09:12:58PM +0500, ??? wrote: > >> + - make reg-tests VTEST_PROGRAM=../vtest/vtest > >> REGTESTS_TYPES=default,bug,devel > >> > > > > let us try that. OK, now pushed. > > I will have a look at "racy" tests. > > Maybe we'll enable them on Github Actions. > > > > > the good thing about Github Actions, it is possible to attach own build > agents. So, if we > have dedicated hardware and we not want to depend on travis-ci neighbours, > it might be an option. That's good to know, even if I doubt we'd need it, at least it opens possibilities. Willy
Re: Disabling regtests in Travis ?
пт, 24 янв. 2020 г. в 20:44, Илья Шипицин :
>
>
> пт, 24 янв. 2020 г. в 20:34, Willy Tarreau :
>
>> On Fri, Jan 24, 2020 at 04:31:07PM +0100, Willy Tarreau wrote:
>> > So I'm proposing that we try a last time to run with
>> > "REGTESTS_TYPES=default,bug,devel"
>>
>> That should probably give this :
>>
>> diff --git a/.travis.yml b/.travis.yml
>> index bf4b82aa98..a82c27327d 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -112,7 +112,7 @@ script:
>>- ./haproxy -vv
>>- if [ "${TRAVIS_OS_NAME}" = "linux" ]; then ldd haproxy; fi
>>- if [ "${TRAVIS_OS_NAME}" = "osx" ]; then otool -L haproxy; fi
>> - - env VTEST_PROGRAM=../vtest/vtest make reg-tests
>> + - make reg-tests VTEST_PROGRAM=../vtest/vtest
>> REGTESTS_TYPES=default,bug,devel
>>
>
> let us try that.
>
> I will have a look at "racy" tests.
> Maybe we'll enable them on Github Actions.
>
>
the good thing about Github Actions, it is possible to attach own build
agents. So, if we
have dedicated hardware and we not want to depend on travis-ci neighbours,
it might be an option.
>
>
>>
>> after_failure:
>>- |
>>
>
Re: Disabling regtests in Travis ?
пт, 24 янв. 2020 г. в 20:34, Willy Tarreau :
> On Fri, Jan 24, 2020 at 04:31:07PM +0100, Willy Tarreau wrote:
> > So I'm proposing that we try a last time to run with
> > "REGTESTS_TYPES=default,bug,devel"
>
> That should probably give this :
>
> diff --git a/.travis.yml b/.travis.yml
> index bf4b82aa98..a82c27327d 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -112,7 +112,7 @@ script:
>- ./haproxy -vv
>- if [ "${TRAVIS_OS_NAME}" = "linux" ]; then ldd haproxy; fi
>- if [ "${TRAVIS_OS_NAME}" = "osx" ]; then otool -L haproxy; fi
> - - env VTEST_PROGRAM=../vtest/vtest make reg-tests
> + - make reg-tests VTEST_PROGRAM=../vtest/vtest
> REGTESTS_TYPES=default,bug,devel
>
let us try that.
I will have a look at "racy" tests.
Maybe we'll enable them on Github Actions.
>
> after_failure:
>- |
>
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
On Fri, Jan 24, 2020 at 01:22:05PM +0100, Emmanuel Hocdet wrote: > > Hi William, > Hello Manu! > > Le 23 janv. 2020 à 16:20, William Lallemand a > > écrit : > > > > That's not a good idea to be able to add a new path to the list each time > > this > > keyword is found, this is not how the configuration works in the global > > section, a new line with the keyword should remove the previous one. > > > > Okay, or refuse the second one. That's really important to be able to overwrite the previous value, because the global section could be superseded. For example in the ALOHA appliances, there is a global.def file which is a configuration file with default values, and these values can be overwritten in the haproxy.cfg. > To be clear, 'issuer-path' can be confused, i could be named > 'issuers-chain-path’. > That might be a good idea. > >> - PEM files into one (e.g. cat cert.pem key.pem > combined.pem). If your > >> CA > >> + PEM files into one (e.g. cat key.pem cert.pem > combined.pem). If your > >> CA > > > > > > I'm not sure to understand why this was changed, maybe by mistake? If not > > could > > you elaborate in the commit message, or split this part in another commit if > > it's not related to the feature. > > > > Indeed, is not related. It’s more optimal when PEM is parsed. > Key is parsed first, Cert next after rewind the BIO at begining. > Oh, that's right. We can probably update the documentation by explaining why it's more efficient in this order. I might be wrong but I think the previous order was mandatory because of the way the certificates were loaded before, because it wasn't using the BIO, but the high level functions of openssl. > > It’s not really magic, it’s based on X509, how certificate are build, and > and how to check(find) an issuer for a certificate (via X509_check_issued > with openssl). > It’s what libssl do to verify certificat, find the appropriate root CA > (loaded via ca-file in > HAproxy). I understand the principle, but my fear is the fact that its a global context only referenced by IDs, and you don't specified which file are used in the configuration. > For certificats generated via the same issuer, there are no reason to not > complete it > with the same chain. At worst, not used the functionality (include a chain > who match the issuer). > I have experienced this for years on our multi-tenant platform. Customers > upload > their certificates (the chain are ignored) and we accept it if we can verify > it. > For that we need, in addition to "ca-certificates » pkg, all intermediate > certificates from > accepted CA (Comodo, LetsEncrypt, CAcert, Cloudflare, …). > When PEM file are generated for HAproxy, the chain of issuers is construct > from > all this intermediate certificates. This guarantees to have a correct chain > for all certificates, > and chain can be updated automatically. > When intermediate certificates repository is updated (by humans) we can have > root > certificate (detected and ignored), or more than one issuer for a certificate > (sha1/sha256 ...): > the better (valid) issuer is chosen. > (1) For haproxy, I didn't want to do that initially, because its’ tricky and > too "magic ». Add such > feature without change the initial behaviour: > Have PEM with Key+Cert+Chain or have Key+Cert and complete the Chain (via the > start of > the chain - the issuer -). > Ok I understand your usage better, it completely makes sense. So the provider will fill itself the directory with trusted files, this is not something which will be filled by the customer. > > Maybe we should use a keyword on the bind line to load it instead of this, > > like > > `ssl crt foo.pem issuer bar.pem` and this keyword could be applied to a lot > > of certificates by using the default-server keyword. > > > (default-server -> ssl-default-bind-options) > (‘issuer’ -> ‘issuers-chain’ to be less confused) > I don’t like that very much. We can have a lot of certificats with different > issuer, and only crt-list > with this issuers-chain param could do the job. It’s very unwieldy. On the > other hand it takes > away any discovery, It might be more in line with how HAproxy deal with ssl > files. That's my concern, but with your explanations I understand that the usage is not really compatible with the way haproxy deals with ssl files. But we should handle it the same way as we plan to handle the certificate directories. > It could looks like ca-file, but ca-file is a PEM who can contient all > CA-root. > A issuers-file could have all issuers in a raw, but we need to extract the > good chain > from that file (1). To update issuers-file payload could be heavy. > > > - it looks like your code is only filling the chain with one certificate, > > shouldn't it try to resolve the whole chain ? > > > cf (1) > > >> Subject: [PATCH 2/2] MINOR: ssl/cli: 'set ssl issuer' load an issuer > >> certificate from the CLI > >> > >>$ echo -e "set ssl issuer
Re: Disabling regtests in Travis ?
On Fri, Jan 24, 2020 at 04:31:07PM +0100, Willy Tarreau wrote:
> So I'm proposing that we try a last time to run with
> "REGTESTS_TYPES=default,bug,devel"
That should probably give this :
diff --git a/.travis.yml b/.travis.yml
index bf4b82aa98..a82c27327d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -112,7 +112,7 @@ script:
- ./haproxy -vv
- if [ "${TRAVIS_OS_NAME}" = "linux" ]; then ldd haproxy; fi
- if [ "${TRAVIS_OS_NAME}" = "osx" ]; then otool -L haproxy; fi
- - env VTEST_PROGRAM=../vtest/vtest make reg-tests
+ - make reg-tests VTEST_PROGRAM=../vtest/vtest
REGTESTS_TYPES=default,bug,devel
after_failure:
- |
Disabling regtests in Travis ?
Hi Ilya, I'm really annoyed by the regtests run in travis. It has become impossible for a month or so to get all of them to work at least once. A single build takes roughly 4-10 minutes depending on the hour of the day, and there are multi-second pauses during the regtests, causing vtest to trigger timeouts, and logs to be reported as empty or invalid. The ones failing most often are precisely those timing dependent: - health checks of various sorts (when the server doesn't respond for multiple seconds because the VM swaps, for sure it cannot work) - peers synchronization (the "show table" command that's already delayed never grasps the synchronized data because the network transfer has not yet occurred between the two instances) So I'm proposing that we try a last time to run with "REGTESTS_TYPES=default,bug,devel" and if it continues to report 100% false positives, that we totally disable them, because quite frankly, I've now got so much used to seeing red there that I'm getting anxious when it's green, in fear of having broken something. The real value of travis is first and foremost to quickly show if we broke something and we've lost this value quite a while ago already by adding so much noise with the tests. What do you think ? Thanks, Willy
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
Willy, Am 24.01.20 um 15:11 schrieb Willy Tarreau: > Well, I guess this is reasonable. I'm still just not 100% sold to the > syntax yet, so depending on users feedback I don't exclude changing it > again, in which case we'll have to backport it as well so that 2.1->2.2 > remains a smooth upgrade. It shouldn't be a big deal as users running > such non-LTS versions cannot have outdated versions anyway (or they're > trying hard to run into trouble), and know they may face a bit more > rough edges than with LTS ones. > I can work with adjusting my configuration, if necessary. Compiling my own HAProxy comes with a little more effort. So that's entirely fine for me. Thanks Tim Düsterhus
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
On Fri, Jan 24, 2020 at 12:12:26PM +0100, Tim Düsterhus wrote: > Willy, > > Am 24.01.20 um 09:23 schrieb Willy Tarreau: > > On Thu, Jan 23, 2020 at 03:56:29PM +0100, Tim Düsterhus wrote: > >> This might have slipped through the cracks. Can you please take a look? > >> If you don't have the time to look at it right now, just let me know. > > > > OK, now merged. > > Thanks. > > Any chance for a backport to the 2.1 non-LTS stable branch? I'm using > Vincent's Debian Packages, the option would ease deployment for me and > it's roughly 4 months to go for 2.2. Well, I guess this is reasonable. I'm still just not 100% sold to the syntax yet, so depending on users feedback I don't exclude changing it again, in which case we'll have to backport it as well so that 2.1->2.2 remains a smooth upgrade. It shouldn't be a big deal as users running such non-LTS versions cannot have outdated versions anyway (or they're trying hard to run into trouble), and know they may face a bit more rough edges than with LTS ones. So that's backported now. Willy
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
Hi Tim, > Le 23 janv. 2020 à 17:21, Tim Düsterhus a écrit : > > Manu, > > Am 21.01.20 um 12:42 schrieb Emmanuel Hocdet: >> Patches updated, depend on "[PATCH] BUG/MINOR: ssl: >> ssl_sock_load_pem_into_ckch is not consistent" > > Out of curiosity: > >> +issuer-path >> + Assigns a directory to load certificate chain for issuer completion. All >> + files must be in PEM format. For certificates loaded with "crt" or >> "crt-list", >> + if certificate chain is not included in PEM (also commonly known as >> intermediate >> + certificate), haproxy will complete chain if issuer match the first >> certificate >> + of the chain loaded with "issuer-path". "issuer-path" directive can be set >> + several times. > > Will HAProxy complete the chain if multiple intermediate certificates > are required? > Patch don’t do that. > Consider this: > > Root CA -> Intermediate CA -> Intermediate CB -> End Certificate > Usually, Root CA should not be include in the chain. > I configure `issuer-path` to a directory that contains the following > certificates: > > - Root CA > - Intermediate CA > - Intermediate CB > You should have a file with: - intermediate CB + intermediate CA > Then I configure a `crt` pointing to a file containing only the End > Certificate. > > What will HAProxy send to the client? > End Certificate + intermediate CB + intermediate CA The same as if you have crt with End Certificate + intermediate CB + intermediate CA ++ Manu
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
Hi William,
> Le 23 janv. 2020 à 16:20, William Lallemand a écrit :
>
> On Tue, Jan 21, 2020 at 12:42:04PM +0100, Emmanuel Hocdet wrote:
>> Hi,
>>
>> Patches updated, depend on "[PATCH] BUG/MINOR: ssl:
>> ssl_sock_load_pem_into_ckch is not consistent"
>>
>
> Hello,
>
> It could be great to share more of the certificates in memory, but some
> points are
> confusing in my opinion:
>
>> +issuer-path
>> + Assigns a directory to load certificate chain for issuer completion. All
>> + files must be in PEM format. For certificates loaded with "crt" or
>> "crt-list",
>> + if certificate chain is not included in PEM (also commonly known as
>> intermediate
>> + certificate), haproxy will complete chain if issuer match the first
>> certificate
>> + of the chain loaded with "issuer-path". "issuer-path" directive can be set
>> + several times.
>> +
>
> That's not a good idea to be able to add a new path to the list each time this
> keyword is found, this is not how the configuration works in the global
> section, a new line with the keyword should remove the previous one.
>
Okay, or refuse the second one.
To be clear, 'issuer-path' can be confused, i could be named
'issuers-chain-path’.
>> - PEM files into one (e.g. cat cert.pem key.pem > combined.pem). If your CA
>> + PEM files into one (e.g. cat key.pem cert.pem > combined.pem). If your CA
>
>
> I'm not sure to understand why this was changed, maybe by mistake? If not
> could
> you elaborate in the commit message, or split this part in another commit if
> it's not related to the feature.
>
Indeed, is not related. It’s more optimal when PEM is parsed.
Key is parsed first, Cert next after rewind the BIO at begining.
>> +/* Find Certificate Chain in global */
>> +if (chain == NULL) {
>> +AUTHORITY_KEYID *akid;
>> +akid = X509_get_ext_d2i(cert, NID_authority_key_identifier,
>> NULL, NULL);
>> +if (akid) {
>> +struct issuer_chain *issuer;
>> +struct eb64_node *node;
>> +u64 hk;
>> +hk = XXH64(ASN1_STRING_get0_data(akid->keyid),
>> ASN1_STRING_length(akid->keyid), 0);
>> +for (node = eb64_lookup(_ssl.cert_issuer_tree,
>> hk); node; node = eb64_next(node)) {
>> +issuer = container_of(node, typeof(*issuer),
>> node);
>> +if
>> (X509_check_issued(sk_X509_value(issuer->chain, 0), cert) == X509_V_OK) {
>> +chain =
>> X509_chain_up_ref(issuer->chain);
>> +break;
>> +}
>> +}
>> +AUTHORITY_KEYID_free(akid);
>> +}
>> +}
>
> I'm not sure about this, what's bothering me is that the certificate is not
> found using a filename, but only found by looking for the SKID. So it's kind
> of
> magic because it's not named in the configuration. It can be bothering if you
> don't want to complete the chain for some of the certificates, or if you have
> a
> multi-tenant configuration.
>
It’s not really magic, it’s based on X509, how certificate are build, and
and how to check(find) an issuer for a certificate (via X509_check_issued with
openssl).
It’s what libssl do to verify certificat, find the appropriate root CA (loaded
via ca-file in
HAproxy).
For certificats generated via the same issuer, there are no reason to not
complete it
with the same chain. At worst, not used the functionality (include a chain who
match the issuer).
I have experienced this for years on our multi-tenant platform. Customers
upload
their certificates (the chain are ignored) and we accept it if we can verify it.
For that we need, in addition to "ca-certificates » pkg, all intermediate
certificates from
accepted CA (Comodo, LetsEncrypt, CAcert, Cloudflare, …).
When PEM file are generated for HAproxy, the chain of issuers is construct from
all this intermediate certificates. This guarantees to have a correct chain for
all certificates,
and chain can be updated automatically.
When intermediate certificates repository is updated (by humans) we can have
root
certificate (detected and ignored), or more than one issuer for a certificate
(sha1/sha256 ...):
the better (valid) issuer is chosen.
(1) For haproxy, I didn't want to do that initially, because its’ tricky and
too "magic ». Add such
feature without change the initial behaviour:
Have PEM with Key+Cert+Chain or have Key+Cert and complete the Chain (via the
start of
the chain - the issuer -).
> Maybe we should use a keyword on the bind line to load it instead of this,
> like
> `ssl crt foo.pem issuer bar.pem` and this keyword could be applied to a lot
> of certificates by using the default-server keyword.
>
(default-server -> ssl-default-bind-options)
(‘issuer’ -> ‘issuers-chain’ to be less confused)
I don’t like that very much. We can have a lot of
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
Willy, Am 24.01.20 um 09:23 schrieb Willy Tarreau: > On Thu, Jan 23, 2020 at 03:56:29PM +0100, Tim Düsterhus wrote: >> This might have slipped through the cracks. Can you please take a look? >> If you don't have the time to look at it right now, just let me know. > > OK, now merged. Thanks. Any chance for a backport to the 2.1 non-LTS stable branch? I'm using Vincent's Debian Packages, the option would ease deployment for me and it's roughly 4 months to go for 2.2. Best regards Tim Düsterhus
Re: Recommendations for deleting headers by regexp in 2.x?
On Fri, Jan 24, 2020 at 10:26:34AM +0100, Christopher Faulet wrote: > Le 24/01/2020 à 09:17, Willy Tarreau a écrit : > > On Fri, Jan 24, 2020 at 08:28:33AM +0100, Christopher Faulet wrote: > > > Le 23/01/2020 à 19:59, James Brown a écrit : > > > > I spent a couple of minutes and made the attached (pretty bad) patch to > > > > add a del-header-by-prefix. > > > > > > > > > > Just an idea. Instead of adding a new action, it could be cleaner to > > > extend > > > the del-header action adding some keywords. Something like: > > > > > >http-request del-header begin-with > > >http-request del-header end-with > > >http-request del-header match > > > > > > It could be also extended to replace-header and replace-value actions. > > > > I would also prefer to extend existing syntax, however it's problematic > > to insert optional words *before* arguments. This will complicate the > > parsing, and even config manipulation scripts. > > > > That's why I thought we could instead just append an extra optional > > keyword appended after the name, eventhough it's less elegant. > > > > From the configuration parsing point of view, it is more or less the same > thing. You must test if the second argument is defined or not. And in fact, > moving it after the header name is not a "better" solution because there is > an optional condition too at the end. So this one will not be the last one. No, it's more complicated this way because you have to check each and every word to figure the syntax. Example: how do you mention that you want to remove the header field matching regex "unless" ? You'd have to do this : http-request del-header match unless And it's ambiguous, as it can either mean : - delete header name "match" unless a condition which needs to be parsed, and once figured invalid, you can roll back ; - delete header described by regex "unless" with no condition When you do it the other way around it's way easier, because the name always being the first argument makes sure the second one is limited to a small subset (match/prefix/if/unless for example): http-request del-header unless match A variant of this could be to use the same syntax as the options we already use on ACL matches, which are "-m reg", "-m beg", "-m end". But these will also need to be placed after to avoid the same ambiguity (since "-m" is a token hence a valid header name). That would give for example : http-request del-header server http-request del-header x-private- -m beg http-request del-header x-.*company -m reg http-request del-header -tracea -m end Willy
Re: arm64 builds?
On Fri, Jan 24, 2020 at 10:31:14AM +0100, Willy Tarreau wrote:
> This is really annoying, so much time spent dealing with bogus warnings
> is really irritating. I'm going to work around it with a cast to unsigned
> short.
>
> The worst part of it is that if it emits such a warning, who knows how valid
> is the code that it will produce given that it doesn't even seem to figure
> the code path :-(
So I could isolate the minimum code to reproduce the bug:
#include
int nbt = 64;
struct srv {
void *a[5];
int **b;
};
void check_config_validity4(struct srv *srv, unsigned int *v)
{
int i;
for (i = 0; i < nbt; i++)
(*v)++;
srv->b = calloc(/*(unsigned int)*/nbt, sizeof(*srv->b));
for (i = 0; i < nbt; i++)
srv->b[i] = 0;
}
Just change the fields ordering in "struct srv" or the size of field "a"
and the bug disappears. Remove any of the loops or make it not depend on
nbt and the bug disappears. Casting nbt to unsigned has no effect. It
always emits the same warning as seen, and only on 32-bits.
When I have a bit of spare time I may file a bug to gcc with this excerpt,
unless someone wants to handle it, of course. Even once fixed it will
probably not flow down to distros given that it's not critical, so we'll
have to work around it as we can.
Willy
Re: arm64 builds?
On Fri, Jan 24, 2020 at 07:12:49AM +0500, ??? wrote: > > it was ubuntu 18.04 + gcc8, I'll try 19.10 + gcc9 > > > > gcc9 produces the same warning OK, I've put my hands on an ubuntu-18.04 + gcc8 32 bits. And now I can confirm it definitely is a gcc bug. The code is the same as a few lines above. The cast was added a first time to shut it up but it re-appears. And if you just change the condition in the "for" loop a few lines above that *walks* through global.nbthread, the warning disappears. Gcc seems to believe that we cannot reach that point if we enter the loop, which is obviously wrong. This is really annoying, so much time spent dealing with bogus warnings is really irritating. I'm going to work around it with a cast to unsigned short. The worst part of it is that if it emits such a warning, who knows how valid is the code that it will produce given that it doesn't even seem to figure the code path :-( Willy
Re: Recommendations for deleting headers by regexp in 2.x?
Le 24/01/2020 à 09:17, Willy Tarreau a écrit : On Fri, Jan 24, 2020 at 08:28:33AM +0100, Christopher Faulet wrote: Le 23/01/2020 à 19:59, James Brown a écrit : I spent a couple of minutes and made the attached (pretty bad) patch to add a del-header-by-prefix. Just an idea. Instead of adding a new action, it could be cleaner to extend the del-header action adding some keywords. Something like: http-request del-header begin-with http-request del-header end-with http-request del-header match It could be also extended to replace-header and replace-value actions. I would also prefer to extend existing syntax, however it's problematic to insert optional words *before* arguments. This will complicate the parsing, and even config manipulation scripts. That's why I thought we could instead just append an extra optional keyword appended after the name, eventhough it's less elegant. From the configuration parsing point of view, it is more or less the same thing. You must test if the second argument is defined or not. And in fact, moving it after the header name is not a "better" solution because there is an optional condition too at the end. So this one will not be the last one. For a configuration script, I hope this is easily manageable. Otherwise, I don't know how it could expect to handle all the complexity of the HAProxy configuration. -- Christopher Faulet
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
On Thu, Jan 23, 2020 at 03:56:29PM +0100, Tim Düsterhus wrote: > This might have slipped through the cracks. Can you please take a look? > If you don't have the time to look at it right now, just let me know. OK, now merged. thanks, Willy
Re: Recommendations for deleting headers by regexp in 2.x?
On Fri, Jan 24, 2020 at 08:28:33AM +0100, Christopher Faulet wrote: > Le 23/01/2020 à 19:59, James Brown a écrit : > > I spent a couple of minutes and made the attached (pretty bad) patch to > > add a del-header-by-prefix. > > > > Just an idea. Instead of adding a new action, it could be cleaner to extend > the del-header action adding some keywords. Something like: > > http-request del-header begin-with > http-request del-header end-with > http-request del-header match > > It could be also extended to replace-header and replace-value actions. I would also prefer to extend existing syntax, however it's problematic to insert optional words *before* arguments. This will complicate the parsing, and even config manipulation scripts. That's why I thought we could instead just append an extra optional keyword appended after the name, eventhough it's less elegant. Willy
Re: Recommendations for deleting headers by regexp in 2.x?
+1 Jan 24, 2020 8:28:33 AM Christopher Faulet : > Le 23/01/2020 à 19:59, James Brown a écrit : > > > I spent a couple of minutes and made the attached (pretty bad) patch to add > > a > > del-header-by-prefix. > > > > > > Just an idea. Instead of adding a new action, it could be cleaner to extend > the > del-header action adding some keywords. Something like: > > http-request del-header begin-with > http-request del-header end-with > http-request del-header match > > It could be also extended to replace-header and replace-value actions. > > Just my 2 cents, > -- > Christopher Faulet >
