Re: [cache] allow caching of OPTIONS request
Hi Baptiste, On Mon, Aug 12, 2019 at 09:35:56PM +0200, Baptiste wrote: > The use case is to avoid too many requests hitting an application server > for "preflight requests". But does this *really* happen to a point of being a concern with OPTIONS requests ? I mean, if OPTIONS represent a small percentage of the traffic I'd rather not start to hack around the standards and regret in 2 versions later... > It seems it owns its own header for caching: > https://www.w3.org/TR/cors/#access-control-max-age-response-header. > Some description here: https://www.w3.org/TR/cors/#preflight-result-cache-0 But all this spec is explicitly for user-agents and not at all for intermediaries. And it doesn't make use of any single Cache-Control header field, it solely uses its own set of headers precisely to avoid mixing the two! And it doesn't suggest to violate the HTTP standards. > I do agree we should disable this by default and add an option > "enable-caching-cors-responses" to enable it on demand and clearly state in > the doc that this is not RFC compliant. > Let me know if that is ok for you. I still feel extremely uncomfortable with this because given that it requires to violate the basic standards to achieve something that is expected to be normal, that smells strongly like there is a wrong assumption somewhere in the chain, either regarding how it's being used or about some requirements. If you don't mind I'd rather bring the question on the HTTP working group to ask if we're missing something obvious or if user-agents suddenly decided to break the internet by purposely making non- cacheable requests, which is totally contrary to their tradition. As you know we've known a period many years ago where people used to say "I inserted haproxy and my application stopped working". Now these days are over (the badmouth will say haproxy stopped working) in main part because we took care of properly dealing with the standards. And clearly I'm extremely cautious not to revive these bad memories. Thanks, Willy
Re: [cache] allow caching of OPTIONS request
On Mon, Aug 12, 2019 at 8:14 AM Willy Tarreau wrote: > Guys, > > On Wed, Aug 07, 2019 at 02:07:09PM +0200, Baptiste wrote: > > Hi Vincent, > > > > HAProxy does not follow the max-age in the Cache-Control anyway. > > I know it's a bit late but I'm having an objection against this change. > The reason is simple, OPTIONS is explicitly documented as being > non-cacheable : https://tools.ietf.org/html/rfc7231#section-4.3.7 > > So not only by implementing it we're going to badly break a number > of properly running applications, but in addition we cannot expect > any cache-control from the server in response to an OPTIONS request > precisely because this is forbidden by the HTTP standard. > > When I search for OPTIONS and cache on the net, I only find AWS's > Cloudfront which offers an option to enable it, and a number of > feature requests responded to by "don't do that you're wrong". So > at the very least we need to disable this by default, and possibly > condition it with a well visible option such as "yes-i-know-i-am- > breaking-the-cache-and-promise-never-to-file-a-bug-report" but what > would be better would be to understand the exact use case and why it > is considered to be valid despite being a blatant violation of the > HTTP standard! History tells us that purposely violating standards > only happens for bad reasons and systematically results in security > issues. > > Thanks, > Willy > Hi Willy, The use case is to avoid too many requests hitting an application server for "preflight requests". It seems it owns its own header for caching: https://www.w3.org/TR/cors/#access-control-max-age-response-header. Some description here: https://www.w3.org/TR/cors/#preflight-result-cache-0 I do agree we should disable this by default and add an option "enable-caching-cors-responses" to enable it on demand and clearly state in the doc that this is not RFC compliant. Let me know if that is ok for you. Baptiste
Re: [PATCH] BUG/MINOR: Fix prometheus '# TYPE' and '# HELP' headers
Le 07/08/2019 à 17:45, Anthonin Bonnefoy a écrit :
From: Anthonin Bonnefoy
Prometheus protocol defines HELP and TYPE as a token after the '#' and
the space after the '#' is necessary.
This is expected in the prometheus python client for example
(https://github.com/prometheus/client_python/blob/a8f5c80f651ea570577c364203e0edbef67db727/prometheus_client/parser.py#L194)
and the missing space is breaking the parsing of metrics' type.
---
contrib/prometheus-exporter/service-prometheus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/prometheus-exporter/service-prometheus.c
b/contrib/prometheus-exporter/service-prometheus.c
index 67914f602..9b9ef2ea8 100644
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -1126,11 +1126,11 @@ static int promex_dump_metric_header(struct appctx
*appctx, struct htx *htx,
types = promex_st_metric_types;
}
- if (istcat(out, ist("#HELP "), max) == -1 ||
+ if (istcat(out, ist("# HELP "), max) == -1 ||
istcat(out, name, max) == -1 ||
istcat(out, ist(" "), max) == -1 ||
istcat(out, desc[appctx->st2], max) == -1 ||
- istcat(out, ist("\n#TYPE "), max) == -1 ||
+ istcat(out, ist("\n# TYPE "), max) == -1 ||
istcat(out, name, max) == -1 ||
istcat(out, ist(" "), max) == -1 ||
istcat(out, types[appctx->st2], max) == -1 ||
Thanks, merged now.
--
Christopher Faulet
Re: [cache] allow caching of OPTIONS request
Guys, On Wed, Aug 07, 2019 at 02:07:09PM +0200, Baptiste wrote: > Hi Vincent, > > HAProxy does not follow the max-age in the Cache-Control anyway. I know it's a bit late but I'm having an objection against this change. The reason is simple, OPTIONS is explicitly documented as being non-cacheable : https://tools.ietf.org/html/rfc7231#section-4.3.7 So not only by implementing it we're going to badly break a number of properly running applications, but in addition we cannot expect any cache-control from the server in response to an OPTIONS request precisely because this is forbidden by the HTTP standard. When I search for OPTIONS and cache on the net, I only find AWS's Cloudfront which offers an option to enable it, and a number of feature requests responded to by "don't do that you're wrong". So at the very least we need to disable this by default, and possibly condition it with a well visible option such as "yes-i-know-i-am- breaking-the-cache-and-promise-never-to-file-a-bug-report" but what would be better would be to understand the exact use case and why it is considered to be valid despite being a blatant violation of the HTTP standard! History tells us that purposely violating standards only happens for bad reasons and systematically results in security issues. Thanks, Willy
Re: [PATCH] BUG/MINOR: lua: fix setting netfilter mark
On Sun, Aug 11, 2019 at 06:03:45PM +0200, Lukas Tribus wrote:
> In the REORG of commit 1a18b5414 ("REORG: connection: centralize the
> conn_set_{tos,mark,quickack} functions") a bug was introduced by
> calling conn_set_tos instead of conn_set_mark.
>
> This was reported in issue #212
>
> This should be backported to 1.9 and 2.0.
Merged, thank you Lukas!
Willy
