Enable header-based proactive content negotiation with optimizations and opt-outs available.

First, I appreciate the initial work-up on this and the thoughtful consideration of reasons *not* to do this, however I think I may have uncovered a use case for supporting this header in the limited sense that sometimes you may *need* to do so for technical reasons.

I came to this issue via #3085360: RouteProvider::getRouteCollectionForRequest() can poison query string of next request, which in short says that you get some buggy, unintuitive behaviour from the route collection cache if an error is handled for a json:api request, but before the request format is set to api_json. That is, an authentication exception will be handled with the request format set to html (the default) and the exception route will be cached with some query strings added. Those query strings, when set on a subsequent authorized request will then make that request fail json:api's validation.

That issue would be solved if the request format can be set sooner. In the case of json_api, spec compliance means supporting only a subset of query parameter keys, and _format is not on the list.

As a quick history review, we use _format because UA and reverse proxy support for the Accept header is spotty. The pattern is actually inherited from Symfony, where the Request object can explicitly set its format from the same query param.

But still, in some cases (most notably json:api, but probably others?) we need the ability to use the Accept header. After some consultation with @gabesullice on Slack, we think this can unlock some blocked issues elsewhere. Since it's possible that this will cause unexpected behaviour behind certain infrastructure, this patch goes a bit further than the original in making this opt-in functionality. We also continue to respect the _format query string over the header, and will only use the header when it is unambiguous - that is, it matches exactly a registered mimetype.

No tests yet, but I think they should be easy enough to add, and we shouldn't fail anything at the moment since this is opt-in.

This would also resolve #2725435: Remove outdated @todo pointing to #2364011.

Rubber-ducking a few questions:

Is there any reason why it might also be necessary to specify a shortlist of headers that will be allowed to get matched from Accept? E.g., make this ONLY match a specified list of headers? That would further narrow this to a specific kind of request. We could make the allowed value of the setting either a boolean OR an array of allowed mimetypes?

Does the issue regarding the poisoning of the route collection cache still need a separate mitigation? That is, it's still broken if you don't implement this.

@bradjones1++ for an excellent summary in #6.

Also after an initial pass, that solution looks really nice overall.

Is there any reason why it might also be necessary to specify a shortlist of headers that will be allowed to get matched from Accept? E.g., make this ONLY match a specified list of headers?

I don't think so. The only reason that I can think of would be to avoid the risk of cache poisoning by a malicious UA requesting random media types via the Accept header. I don't think that's a risk because this patch is doing $request->getFormat($mimeType), which narrow the potential cache variation down to only the registered list of formats. Am I missing some other reason to do so?

On a similar note, I am wondering if we want the settings variable to be something like:

$settings['use_http_accept_header_negotiation'] = [
  'accept' => TRUE,
  'accept-language' => TRUE,
  'accept-encoding' => FALSE,
]

So that we can more granularly support content negotiation. F.e. I know certain CDNs respect some of the accept-* headers, but not all of them.

Does the issue regarding the poisoning of the route collection cache still need a separate mitigation? That is, it's still broken if you don't implement this.

If this resolves the issue, then no, I don't think so. AFAIU, the issue is specific to JSON:API's error WRT the disallowed query parameter _format. Since all JSON:API UA's MUST send the Accept header, this issue should fix it in all cases as long as the $settings is enabled. Since this is such an edge-case, I think it's okay to leave it there.

1. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -89,6 +100,12 @@ protected function getContentType(Request $request) {
   +    else if (Settings::get('use_http_accept_header_negotiation', FALSE)) {
   

   Nit: elseif not else if. Though, I think we can just do an extra newline + if since the first condition will has a return in it.

2. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -89,6 +100,12 @@ protected function getContentType(Request $request) {
   +      if (count($accept) === 1 && $format = $request->getFormat($accept[0])) {
   

   I know that in Slack we talked about only doing this iff there was only one request header, but is that really necessary? I'm not convinced any more.

   Also, super nitty, but maybe just do reset($accept) instead of $accept[0]. I've been bitten by weird, unexpected indices too many times.

3. +++ b/sites/default/default.settings.php
   @@ -742,6 +742,15 @@
   +/**
   + * Accept header content negotiation.
   + *
   + * This is used to inform the HTTP content type negotiation middleware it should
   + * use the HTTP Accept header as a potential source for setting the request
   + * format.
   + */
   +# $settings['use_http_accept_header_negotiation'] = TRUE;
   

   I wonder if we should immediately add a Drupal 9 followup to make this the default and make FALSE the opt-in behavior. IMO, Drupal should do the right thing with respect to HTTP and provide this as a mechanism for user's with misbehaving infrastructure opt-in to the workaround instead.

Nice work! Leaving as NR for others to review this. I'll ping @Wim Leers and @effulgentsia to have them take a look and weigh in also.

On a similar note, I am wondering if we want the settings variable to be something like:

Obviously, I think this patch should only add the accept header at first. We can add the other headers later, as necessary.

Since all JSON:API UA's MUST send the Accept header,

Reread the spec, I guess it's not required. But Drupal's JSON:API module did make this a requirement: https://www.drupal.org/node/3013494

Very, very nice work. 👏

I'm impressed with the detective work you did, across multiple issues, across multiple subsystems in core. 😳🙏

I wonder if we should immediately add a Drupal 9 followup to make this the default and make FALSE the opt-in behavior.

I wanted to ask something similar, but different. Why do we even need to make this a setting if it's risk-free? It's gonna be strange that error responses behave differently (and more sensible) on the miniscule percentage of sites that will turn this setting on.

I understand it's for minimizing risk. But … can you articulate what the risk would be, what the consequences would be?

I wanted to ask something similar, but different. Why do we even need to make this a setting if it's risk-free? It's gonna be strange that error responses behave differently (and more sensible) on the miniscule percentage of sites that will turn this setting on.

I don't think it's risk-free. If you enabled this with REST module on and send a Accept: application/hal+json request to /node/1 then visit /node/1 with your browser, you'll get a JSON response if you have a misbehaving intermediary.

Maybe you're saying new installs, even of Drupal 8, should have this set to TRUE by default, but existing installs should have a BC setting this to FALSE?

@gabesullice - Thanks for the thoughtful review - I actually had reset() in there originally but didn't know if it was too clever. Your comments are all great.

I do have a question though about the json:api spec requirement vis-a-vis the linked issue that led me here. One, if the client is not compliant (or during testing you forget the Accept header, or whatever) then you get the cache poisoning issue.

More specifically, json:api in Drupal, as currently implemented, does not enforce this requirement if it is indeed a hard and fast validation constraint. As evidenced by the fact we're having this discussion to begin with, the Accept header is nowhere consulted in setting the format to api_json or validating the request. The request validator does other things, but it doesn't check the header. We just assume that if you're hitting a jsonapi path, that's your request format. See Drupal\jsonapi\Routing\EarlyFormatSetter.

More specifically, the spec reads:

Clients MUST send all JSON:API data in request documents

...but to me a request document is a payload; what about a GET? Maybe I'm missing the definition of a "document" elsewhere to mean, any request to json:api, even one without a document payload?

Since you're a spec maintainer, it would be great to get that answer from you. If the answer is yes, header required all the time, then I think we can maybe remove Drupal\jsonapi\Routing\EarlyFormatSetter and perhaps use some earlier-firing middleware or event to validate the request. But I do think the cache poisoning is an issue since we want to catch non-conforming requests and offer a solution (much like the current validator), not potentially poison the cache.

So digging into this further - per Gabe's note on #10 (which I originally missed) - the json:api Accept header is not required in the spec for GET... but it was added as a format-setting requirement in #3009596: JSON API 2.x responses always result in a Page Cache MISS... but then removed per the commit to core - see this comment and interdiff.

In that light, this is actually a regression from the core integration? In so far as jsonapi 2.x sets the format with early-firing middleware from a property of the request, but it was replaced with a route filter that instead set the request format based on the route.

Fresh patch per the feedback from above, also using Symfony\Component\HttpFoundation\AcceptHeader for fetching the values from the header, partly to use existing tools and second, if this does get expanded in Drupal 9 to allow for other types of content-type negotiation, etc., then it's already in here to use the Symfony component. The way the patch was written originally does work for our present requirement (only match if there's only one value and it explicitly matches a known mimetype) but I think this is more semantic than doing a match against the raw Accept header which may included comma-delimited multiple values. So we use AcceptHeader::fromString() to parse it, then see if it's one value, and further if it's known.

The test failure was on known scaffold files (I'm actually going to include a patch without the change to settings.default.php, so I can still bring this with Composer...) so we can address that once the language is agreed-upon.

Further nits and expansions:

Since it looks like the requirement for requests to contain the appropriate Accept header was added in contrib 2.x, then removed in the commit to core, do we want to make a follow-up to respect the spec to require the correct value on document requests but not GET? I'm not sure if this qualifies as BC break since right now Drupal is more lax than the spec, or...?

Also re:

I wonder if we should immediately add a Drupal 9 followup to make this the default and make FALSE the opt-in behavior. IMO, Drupal should do the "right" thing with respect to HTTP and provide this as a mechanism for user's with misbehaving infrastructure opt-in to the workaround instead.

I like the idea of Drupal not twisting into too much of a contortion to address specific reverse proxies, though not all of them are always going to be under the control of the site owner (corporate proxies?) and I wonder if the upgrade path on this is a little confusing?

I also re-opened the cache poisoning issue since it will not entirely be resolved by this patch (that is, if you send a malformed request you still poison, see note above) - though I think this was foreseen in part by Wim during the core merge process:

Some error responses happen before or during routing, and at that time the JSON:API format hasn't been negotiated yet. So … you get a HTML response.

And that was addressed by changing the weight of the format setter, but it still leaves exceptions before routing. But we still need this issue, too :-)

This one is enough

Tests! A test-only patch, interdiff and full patch.

So close - Settings is tough to mock since it's a static, so I unset the test settings when we're done; it does however reveal that the mocked Request in ::testHandle() doesn't have a header parameter set, so adding a HeaderBag there for future-proofing.

The test-only patch above is still basically valid, so not re-adding it here.

Code-only (no settings file changes) patch for use with composer-patches and drupal-project.

@bradjones1, this looks great! Thanks!

I reviewed and then addressed all my concerns.

I went a step further than your patch by setting the `Vary` header under all the circumstances where it will be necessary that I can think of. I also adjusted everything to work based primarily off of a container param instead of a settings.php value. Now, the value is only used as a temporary BC measure.

Finally, I went ahead and tried to add some sensible optimizations to keep cache variation to a minimum. For example, I only set the vary: accept header on responses when there the serializer.formats container parameter is available (which is basically a test for whether the serialization module is enabled).

I think I thought of everything I could, but I will not feel confident until this gets some careful scrutiny by @Wim Leers.

1. +++ b/core/assets/scaffold/files/default.settings.php
   @@ -742,6 +742,15 @@
   +# $settings['use_http_accept_header_negotiation'] = TRUE;
   
   +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -2,13 +2,25 @@
   + * utilize the Accept header for content-type negotiation; this behaviour
   

   I added a lot more nuance to this configuration. The gist is that content negotiation is now managed as a container parameter, but this can be opted out of via settings.php.

   The long story is that for existing installs, everything will continue to work as before, because they will not have `$settings['enable_content_negotiation'] = TRUE` defined. New installs will have this defined when the settings file is created during installation. In Drupal 9, we can remove this setting altogether and only respect the values defined in `services.yml`, using defaults when this is not defined (thus making it purely an opt-out/refinement thing).

2. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -2,13 +2,25 @@
   + * For certain applications (json:api, for instance) developers may wish to
   

   Nit: JSON:API (capitalized)

3. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -2,13 +2,25 @@
   + * utilize the Accept header for content-type negotiation; this behaviour
   

   Nit: s/Accept/`accept`: HTTP headers should be lower-cased since HTTP/2 and I'm on a mission to fix the internet! :P See https://blog.yaakov.online/http-2-header-casing/

   Nit: s/content-type/content: "Content negotiation" is a broader concept than just the content-type. See https://tools.ietf.org/html/rfc7231#section-5.3.

1. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -102,7 +117,7 @@ protected function getContentType(Request $request) {
   +    if ($this->contentNegotiationConfig['respect_headers']['accept'] ?? FALSE) {
   

   s/respect_headers/headers

2. +++ b/sites/default/default.services.yml
   @@ -172,3 +172,14 @@ parameters:
   +    # `use_http_content_negotiation_headers` is set to TRUE via settings.php
   

   This should be enable_content_negotation

I created a follow-up to remove the settings.php variable in D9: #3087179: Remove the `enable_content_negotiation` setting from default.settings.php and ContentNegotiationCompilerPass.php

I'd like to get @effulgentsia's opinion as well.

More self-review:

1. +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/ContentNegotiationCompilerPass.php
   @@ -0,0 +1,55 @@
   +    // does not make sense to enable content negotiation via the accept` header.
   

   Missing a backtick before accept`

2. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -2,13 +2,23 @@
   + * in practice, Drupal suggests the use of a _format query parameter for
   + * expressing a content type that would otherwise be sent in an Accept HTTP
   + * header.
   

   I'm not sure that this should be our suggestion. I'd like for us to be optimistic about proxy support, not pessimistic.

3. +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php
   @@ -2,13 +2,23 @@
   + * disabled by configuring the 'http_content_negotiation_headers' setting.
   

   s/http_content_negotiation_headers settings/content_negotiation.config container parameter in services.yml

4. +++ b/core/modules/serialization/src/SerializationServiceProvider.php
   @@ -15,7 +16,9 @@ class SerializationServiceProvider implements ServiceProviderInterface {
   +    // Set a priority of 1 so that this pass runs before
   +    // see \Drupal\Core\DependencyInjection\Compiler\ContentNegotiationCompilerPass
   +    $container->addCompilerPass(new RegisterSerializationClassesCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 1);
   

   Maybe ContentNegotiationCompilerPass should have a priority of -1 instead. Curious what others think.

+++ b/core/assets/scaffold/files/default.settings.php
@@ -742,6 +742,15 @@
+/**
+ * Accept header content negotiation.
+ *
+ * This is used to inform the HTTP content type negotiation middleware it should
+ * use the HTTP Accept header as a potential source for setting the request
+ * format.
+ */
+# $settings['use_http_accept_header_negotiation'] = TRUE;

This works fine for reverse proxies.

But those have never been the problem. Reverse proxies are under control of the site/application/Drupal.

The problem are proxies in front of networks at big corporations, universities, hospitals. Those are out of the control of the application/site/Drupal owner. They may not handle this correctly. And hence cause things to break for all other users on that network interacting with this application.

Now, with the rise of TLS, and the slow rise of HTTP/2 (which de facto requires TLS), this is becoming less and less of a problem. Because these responses are not being cached anyway!

So, in reality, it's only a problem anymore when:

1. there's a crappy proxy
2. the site is using HTTP/1
3. the site is not using TLS
4. the site is serving a HTTP API to anonymous users

All four factors have to apply before problems arise. But it's very hard to quantify this. And without hard data, it's hard to choose to ship this with Drupal core.

Thoughts?

This would be a minor-only change. Since 8.9.x and 9.0.x are now in beta, I'm moving this to 9.1.x. Thanks!

Re-roll and incorporating some of Gabe's self-nits. Also removing the references to deprecated $settings option since we're now D9.

I think caveat emptor is an OK approach re: Wim's notes above re: the situations in which the content negotiation can go sideways. Do we need to somehow make it "more" obvious to site owners that the conditions listed above could be an issue? I think we're rapidly approaching a situation where HTTP/2, TLS, etc. are the norm and the noncompliant edge case here is less significant than it was even a year ago?

Now, with the rise of TLS, and the slow rise of HTTP/2 (which de facto requires TLS), this is becoming less and less of a problem. Because these responses are not being cached anyway!

So, in reality, it's only a problem anymore when:

1. there's a crappy proxy
2. the site is using HTTP/1
3. the site is not using TLS
4. the site is serving a HTTP API to anonymous users

All four factors have to apply before problems arise. But it's very hard to quantify this.

• HTTP/2 is now deployed to ~1/2 of all websites.
• As of October 2020, between 79-98% of all pages loaded in Chrome use HTTPS. If Linux clients are excluded, the numbers jump to 89-98%. (source)
• The Google team "… used private data sources to track the HTTPS state of the top 100 non-Google sites on the Internet. By [their] estimates, these 100 sites account for approximately 25% of all website traffic worldwide." and 97/100 of those sites default to HTTPS. (source)

Therefore, it seems to me that the numbers affected must be vanishingly small.

And without hard data, it's hard to choose to ship this with Drupal core. Thoughts?

I hope the numbers above temper your caution. What this issue proposes is not something radical. It proposes that we permit Drupal to do content negotiation the way that it was designed as far back as 1997! IOW, the burden of proof is not ours. Even so, we can see how rare the scenario you describe must be.

I understand that there is a risk that a very small number of users may see broken sites because of misbehaving caches. That would not be our fault, nor our responsibility. It would be the fault of the IT team managing that network.

This patch permits website owners to opt-out of this behavior if find our default objectionable. With this patch, at least they have a choice. Right now, the only choice is custom code.

It seems to me that the numbers affected must be vanishingly small.

I agree!

I wrote

All four factors have to apply before problems arise. But it's very hard to quantify this. And without hard data, it's hard to choose to ship this with Drupal core.

You just did quantify this! 👏👏🙏

Based on this data, I personally feel comfortable.

It proposes that we permit Drupal to do content negotiation the way that it was designed as far back as 1997! IOW, the burden of proof is not ours.

This is the wrong argument to make, even though I very much share your frustration! Theoretical realities don't help anyone in the real world.

Even so, we can see how rare the scenario you describe must be.

This is the right argument to make. 😊

I understand that there is a risk that a very small number of users may see broken sites because of misbehaving caches. That would not be our fault, nor our responsibility. It would be the fault of the IT team managing that network.

This is also the right argument to make, now that we have the numbers on the side of the theoretically/technically preferable approach. Because now that IT team will be pressured not just by one site "being broken on the organization's network", but because if the theoretical broken Drupal site that uses Accept-header based content negotiation gets reports from these unfortunate users, then the site owner can choose to switch over to HTTPS and … be 100% certain it will work. The IT team cannot cannot in good conscience block HTTPS.

The web is a different world than it was at the time of #2445723: Use the $request format instead of the ContentNegotation., #2364011: [meta] External caches mix up response formats on URLs where content negotiation is in use and #2481453: Implement query parameter based content negotiation as alternative to extensions:

• You started your argument with HTTP/2. That literally was not yet introduced until after #2445723: Use the $request format instead of the ContentNegotation.!
• Only in 2017, a tipping point was reached: most page loads happened using HTTPS. That does not yet mean most sites though: this is highly impacted by the top 100 sites. We're now 3 years further, the number you cite confirm this is now the case.
• Specifically, you site "79–98% in October 2020" — that was 29-45% in March 2015, shortly after #2445723: Use the $request format instead of the ContentNegotation. occurred. This very strongly backs up your argument 💯

So, yes, I agree that the four factors that have to apply before problems arise now make for such a low count in the real world that it totally makes sense to move forward with this.

Another point in favor of this: https://blog.mozilla.org/security/2020/11/17/firefox-83-introduces-https...

I'm tempted to bump this to Major 🤓

I'm being bold and following Wim's suggestion in #45 to make this major. We are building mobile apps that use Drupal as a data store and the _format=json requirement and edge case bugs where you still get an HTML response even though you only accept JSON is irritating - I only just found this issue but this patch hopefully goes some way to fixing that, we are planning to try it out shortly.

The MR needs updating as it mentions Drupal 8.8 but I don't think this will get in until 9.3 now. Also regarding the deprecated setting I think we need to add it to Drupal\Core\Site\Settings::$deprecatedSettings?

Thanks for doing that @longwave. I did a some research into how CDNs treat Vary: Accept and realized that the Accept header isn’t something I want to spend more of my time on since they don’t handle it well. I could see support for true Accept content negotiation as a contrib module. In fact, I think we really should have that as an option for people who can verify that their CDN is doing things correctly. However, proactive content negotiation still isn’t something we should default to in core... which is really disappointing. When I recreated this issue I was very optimistic that the ecosystem had improved since the _format query parameter became a thing.

On the bright side, I found that Drupal core already has a test module somewhere that has a rudimentary proactive content negotiation middleware that could form the basis of a contrib module. If you’re interested in writing that, I’d be more than willing to help!

For your more immediate needs, I recently committed this to the decoupled_menus module. It does content negotiation based on the hostname. For example, one can configure www.example.com to default to HTML and api.example.com to default to the JSON:API format. That means you don’t need the _format query string. It might be helpful for you, but you’ll probably need to patch it to make it refuse to return an HTML response if a JSON one isn’t available. It should fix cases where there is a JSON response and Drupal is incorrectly sending HTML though.

Thinking out loud here: in language module we detect the requested language via various plugins; we allow URL prefix, domain name, querystring parameter, Accept-Language header, etc. Should we offer similar options for content negotiation? It feels like most if not all of those options are equally valid for content types as they are for languages, there can be a similar fallback mechanism, etc.

I'm curious if Gabe can quantify/provide links to his research which leads to the statement, "I did a some research into how CDNs treat Vary: Accept and realized that the Accept header isn’t something I want to spend more of my time on since they don’t handle it well."

It seems to me that the conditions under which header-based content negotiation fails are outlined in this comment led both Gabe and Wim to conclude

What this issue proposes is not something radical. It proposes that we permit Drupal to do content negotiation the way that it was designed as far back as 1997! IOW, the burden of proof is not ours. Even so, we can see how rare the scenario you describe must be.

So, yes, I agree that the four factors that have to apply before problems arise now make for such a low count in the real world that it totally makes sense to move forward with this.

respectively. People have the right to change their mind, but I'm curious what has changed here to basically say this is not a candidate for core and should only be done in contrib. If I understand the conversation thus far, it is the site owner's responsibility to select a well-behaved reverese proxy in front of their application if they wish to use this feature. Further, the concern about forward proxies articulated above is becoming a vanishingly small concern in so far as the historical users of such heavy-handed forward HTTP proxying cannot do so any longer due to the advent of near universal TLS.

As Gabe is doing in decoupled_menus (which reminded me of this issue) it's certainly possible for contrib to accomplish this, and I am willing to split this out into a contrib module if need be. But I think this is definitely something that can/should be addressed in core, even if it has to wait until 10.x, with contrib as a polyfill until then.

From what I can see, Cloudflare does not support Vary at all: https://community.cloudflare.com/t/cloudflare-cdn-cache-to-support-http-...

However other popular CDNs do claim to support it:

AWS: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/Reque...

GCP: https://cloud.google.com/cdn/docs/caching#vary-headers

Fastly: https://developer.fastly.com/reference/http-headers/Vary/

As #51 says I don't think we should choose not to include this in core just because it won't work behind certain CDNs. We don't have to enable it by default (if there are concerns about caching incorrect content types on problematic CDNs), but it could still be an option.

Thanks. So that would change this to opt-in instead of opt-out? But still worth going into core.

Posting a patch for 9.3.x which does not include the changes to the shipped default.* files, for clean application to current projects.

@effulgentsia URL prefixes probably don't make much sense but URL suffixes might. /node/1 can return HTML but /node/1.json can return the same content in JSON?

I'm curious if Gabe can quantify/provide links to his research which leads to the statement, "I did a some research into how CDNs treat Vary: Accept and realized that the Accept header isn’t something I want to spend more of my time on since they don’t handle it well."

@bradjones1, thanks for keeping me accountable ;)

Unfortunately, I lost access to the Google Doc that I used to record my findings when I left Acquia. Perhaps @effulgentsia would be able to recover it, I believe it is owned by @tim.plunkett now. FWIW, it is not a detailed report; it only contains notes that I jotted down as I was testing browser+CDN behavior using this quick-and-dirty tool that I developed: https://github.com/gabesullice/vary-accept-tester (it doesn't have a README, but it is fairly well-commented).

IIRC, the reason I decided that I do not want to spend more time on this issue is because all major browsers and many CDNs do not cache multiple variations of a response and that makes this useless for my needs (i.e. responding with both HTML and JSON:API response from the same URL without forcing an HTTP client to fiddle with linked URLs). However, anyone is free to work on this as they please. "We" can certainly commit some form of support for this to core.

Almost all caches behave "correctly" insofar as their caches will not serve a cached response processed for Accept: A if an incoming request specifies Accept: B. However, they will not simultaneously cache a response for A and B. All browsers that I tested simply overwrite the A-response with the B-response. This has the effect that only one response per-URL can be cached at a time. Many CDNs behave this way as well. This isn't a deal-breaker for single-page apps IMO (since many will only request HTML once and then JSON for all other requests) but it's pretty bad at the CDN layer (see below).

People have the right to change their mind, but I'm curious what has changed here to basically say this is not a candidate for core and should only be done in contrib.

Sorry, I really could have phrased my opinion better. It was not my intention to say that this should never be included it core (which I why I left the issue metadata unchanged). My opinion is that core should not enable proactive content negotiation by default since it can lead to a form of cache poisoning (see below).

Since core already has an example implementation and a test for that example, I think a contrib module wouldn't require much maintenance. The project page would provide a nice place to document when/why it is useful and the infrastructure requirements. It's even possible that content negotiation might become more popular if there's a contrib module for people to blog about than if there is some nondescript configuration option somewhere.

Cache poisoning example

If:

1. you have a site which serves a large volume of traffic,
2. you are using a CDN which has a "one cache item per-URL" policy,
3. and your site returns Vary: Accept by default,

then a malicious actor can easily send a request with Accept: {random-characters} every 10 seconds to continuously invalidate your high-traffic URLs.

Update: Just read the idea to make this configurable like language negotiation. I think that's a very intriguing idea :)

Thank you! A good encapsulation of where we're at. I think it's also worth mentioning that even sites which _do not_ sit entirely behind a reverse proxy can benefit from this. E.g., being able to craft well-behaved headless clients (SPAs, XHR, native apps) to avoid appending ?format=_json to all URLs. Dare I say that's the primary motivation for this, and the CDN compatibility question is a necessary consideration if it's going to be in core.

@gabesullice Thanks for that very illuminating comment!

P.S.: I’ll coordinate with Tim tomorrow :)

@tim.plunkett was able to share the doc with me: https://docs.google.com/document/d/1vvjJiM94wNxvs3krTgaDzJ7-OHncbJjasw4U...

MR for 9.4.x and attaching a patch that applies cleanly to project sites.

Re-roll of #65