Re: [cache] allow caching of OPTIONS request
On Mon, Aug 12, 2019 at 10:19 PM Willy Tarreau wrote: > 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 > Hi Willy, Yes I understand. Would be great to have the feedback from the http working group. In the mean time, if some people here would like to share with Willy and I privately some numbers on what percentage of the traffic do OPTIONS requests represent, this would be helpful. Baptiste
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: [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: [cache] allow caching of OPTIONS request
On Wed, Aug 07, 2019 at 03:20:34PM +0200, Baptiste wrote: > On Wed, Aug 7, 2019 at 3:18 PM William Lallemand > wrote: > > > On Wed, Aug 07, 2019 at 12:38:05PM +0200, Baptiste wrote: > > > Hi there, > > > > > > Please find in attachement a couple of patches to allow caching responses > > > to OPTIONS requests, used in CORS pattern. > > > In modern API where CORS is applied, there may be a bunch of OPTIONS > > > requests coming in to the API servers, so caching these responses will > > > improve API response time and lower the load on the servers. > > > Given that HAProxy does not yet support the Vary header, this means this > > > feature is useful in a single case, when the server send the following > > > header "set access-control-allow-origin: *". > > > > > > William, can you check if my patches look correct, or if this is totally > > > wrong and then I'll open an issue on github for tracking this one. > > > > > > > Looks good to me, pushed in master. > > > > -- > > William Lallemand > > > > Great, thanks! > > Baptiste Don't forget to update the documentation, Thanks -- William Lallemand
Re: [cache] allow caching of OPTIONS request
On Wed, Aug 7, 2019 at 3:18 PM William Lallemand wrote: > On Wed, Aug 07, 2019 at 12:38:05PM +0200, Baptiste wrote: > > Hi there, > > > > Please find in attachement a couple of patches to allow caching responses > > to OPTIONS requests, used in CORS pattern. > > In modern API where CORS is applied, there may be a bunch of OPTIONS > > requests coming in to the API servers, so caching these responses will > > improve API response time and lower the load on the servers. > > Given that HAProxy does not yet support the Vary header, this means this > > feature is useful in a single case, when the server send the following > > header "set access-control-allow-origin: *". > > > > William, can you check if my patches look correct, or if this is totally > > wrong and then I'll open an issue on github for tracking this one. > > > > Looks good to me, pushed in master. > > -- > William Lallemand > Great, thanks! Baptiste
Re: [cache] allow caching of OPTIONS request
On Wed, Aug 07, 2019 at 12:38:05PM +0200, Baptiste wrote: > Hi there, > > Please find in attachement a couple of patches to allow caching responses > to OPTIONS requests, used in CORS pattern. > In modern API where CORS is applied, there may be a bunch of OPTIONS > requests coming in to the API servers, so caching these responses will > improve API response time and lower the load on the servers. > Given that HAProxy does not yet support the Vary header, this means this > feature is useful in a single case, when the server send the following > header "set access-control-allow-origin: *". > > William, can you check if my patches look correct, or if this is totally > wrong and then I'll open an issue on github for tracking this one. > Looks good to me, pushed in master. -- William Lallemand
Re: [cache] allow caching of OPTIONS request
Hi Vincent, HAProxy does not follow the max-age in the Cache-Control anyway. Here is what the configuration would look like: backend X http-request cache-use cors if METH_OPTIONS http-response cache-store cors if METH_OPTIONS cache cors total-max-size 64 max-object-size 1024 max-age 60 You see, the time the object will be cached by HAProxy is defined in your cache storage bucket. Baptiste On Wed, Aug 7, 2019 at 1:47 PM GALLISSOT VINCENT wrote: > Hi there, > > > May I add that, in the CORS implementation, there is a specific header > used for the caching duration: *Access-Control-Max-Age* > > This header is supported by most of browsers and its specification is > available : https://fetch.spec.whatwg.org/#http-access-control-max-age > > One would think of using this header value instead of the well known > Cache-Control header when dealing with CORS and OPTIONS requests. > > Cheers, > Vincent > > -- > *De :* Baptiste > *Envoyé :* mercredi 7 août 2019 12:38 > *À :* HAProxy; William Lallemand > *Objet :* [cache] allow caching of OPTIONS request > > Hi there, > > Please find in attachement a couple of patches to allow caching responses > to OPTIONS requests, used in CORS pattern. > In modern API where CORS is applied, there may be a bunch of OPTIONS > requests coming in to the API servers, so caching these responses will > improve API response time and lower the load on the servers. > Given that HAProxy does not yet support the Vary header, this means this > feature is useful in a single case, when the server send the following > header "set access-control-allow-origin: *". > > William, can you check if my patches look correct, or if this is totally > wrong and then I'll open an issue on github for tracking this one. > > Baptiste >
RE: [cache] allow caching of OPTIONS request
Hi there, May I add that, in the CORS implementation, there is a specific header used for the caching duration: Access-Control-Max-Age This header is supported by most of browsers and its specification is available : https://fetch.spec.whatwg.org/#http-access-control-max-age One would think of using this header value instead of the well known Cache-Control header when dealing with CORS and OPTIONS requests. Cheers, Vincent De : Baptiste Envoyé : mercredi 7 août 2019 12:38 À : HAProxy; William Lallemand Objet : [cache] allow caching of OPTIONS request Hi there, Please find in attachement a couple of patches to allow caching responses to OPTIONS requests, used in CORS pattern. In modern API where CORS is applied, there may be a bunch of OPTIONS requests coming in to the API servers, so caching these responses will improve API response time and lower the load on the servers. Given that HAProxy does not yet support the Vary header, this means this feature is useful in a single case, when the server send the following header "set access-control-allow-origin: *". William, can you check if my patches look correct, or if this is totally wrong and then I'll open an issue on github for tracking this one. Baptiste
[cache] allow caching of OPTIONS request
Hi there,
Please find in attachement a couple of patches to allow caching responses
to OPTIONS requests, used in CORS pattern.
In modern API where CORS is applied, there may be a bunch of OPTIONS
requests coming in to the API servers, so caching these responses will
improve API response time and lower the load on the servers.
Given that HAProxy does not yet support the Vary header, this means this
feature is useful in a single case, when the server send the following
header "set access-control-allow-origin: *".
William, can you check if my patches look correct, or if this is totally
wrong and then I'll open an issue on github for tracking this one.
Baptiste
From b1ed59901522dc32fa112e77c93c9a723ecc2189 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 7 Aug 2019 12:24:36 +0200
Subject: [PATCH 2/2] MINOR: http: allow caching of OPTIONS request
Allow HAProxy to cache responses to OPTIONS HTTP requests.
This is useful in the use case of "Cross-Origin Resource Sharing" (cors)
to cache CORS responses from API servers.
Since HAProxy does not support Vary header for now, this would be only
useful for "access-control-allow-origin: *" use case.
---
src/cache.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index 5b4062384..001532651 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -560,8 +560,8 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
if (!(txn->req.flags & HTTP_MSGF_VER_11))
goto out;
- /* cache only GET method */
- if (txn->meth != HTTP_METH_GET)
+ /* cache only GET or OPTIONS method */
+ if (txn->meth != HTTP_METH_GET && txn->meth != HTTP_METH_OPTIONS)
goto out;
/* cache key was not computed */
@@ -1058,6 +1058,9 @@ int sha1_hosturi(struct stream *s)
ctx.blk = NULL;
switch (txn->meth) {
+ case HTTP_METH_OPTIONS:
+ chunk_memcat(trash, "OPTIONS", 7);
+ break;
case HTTP_METH_HEAD:
case HTTP_METH_GET:
chunk_memcat(trash, "GET", 3);
@@ -1093,10 +1096,10 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
struct cache_flt_conf *cconf = rule->arg.act.p[0];
struct cache *cache = cconf->c.cache;
- /* Ignore cache for HTTP/1.0 requests and for requests other than GET
- * and HEAD */
+ /* Ignore cache for HTTP/1.0 requests and for requests other than GET,
+ * HEAD and OPTIONS */
if (!(txn->req.flags & HTTP_MSGF_VER_11) ||
- (txn->meth != HTTP_METH_GET && txn->meth != HTTP_METH_HEAD))
+ (txn->meth != HTTP_METH_GET && txn->meth != HTTP_METH_HEAD && txn->meth != HTTP_METH_OPTIONS))
txn->flags |= TX_CACHE_IGNORE;
http_check_request_for_cacheability(s, >req);
--
2.17.1
From e3aee8fe302e108e2652842f537dc850978d2e59 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Mon, 5 Aug 2019 16:55:32 +0200
Subject: [PATCH 1/2] MINOR: http: add method to cache hash
Current HTTP cache hash contains only the Host header and the url path.
That said, request method should also be added to the mix to support
caching other request methods on the same URL. IE GET and OPTIONS.
---
src/cache.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index 9cef0cab6..5b4062384 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1041,9 +1041,9 @@ enum act_parse_ret parse_cache_store(const char **args, int *orig_arg, struct pr
return ACT_RET_PRS_OK;
}
-/* This produces a sha1 hash of the concatenation of the first
- * occurrence of the Host header followed by the path component if it
- * begins with a slash ('/'). */
+/* This produces a sha1 hash of the concatenation of the HTTP method,
+ * the first occurrence of the Host header followed by the path component
+ * if it begins with a slash ('/'). */
int sha1_hosturi(struct stream *s)
{
struct http_txn *txn = s->txn;
@@ -1056,6 +1056,16 @@ int sha1_hosturi(struct stream *s)
trash = get_trash_chunk();
ctx.blk = NULL;
+
+ switch (txn->meth) {
+ case HTTP_METH_HEAD:
+ case HTTP_METH_GET:
+ chunk_memcat(trash, "GET", 3);
+ break;
+ default:
+ return 0;
+ }
+
if (!http_find_header(htx, ist("Host"), , 0))
return 0;
chunk_memcat(trash, ctx.value.ptr, ctx.value.len);
--
2.17.1
