`@netlify/config`: Multiple headers with different cases should not be permitted in netlify.toml

[mheffner]

Describe the bug

A netlify.toml with a headers section containing the same header repeated more than once will fail to parse, which appears to track the correct behavior. However, if you change the case of the second header field, but with the same header name, it will parse successfully. Header names are case insensitive, so in theory they should be handled the same.

To Reproduce

Create a netlify.toml with the following header block:

[[headers]]
  for = "/headers/test.json"
  [headers.values]
    X-Header-Test = "a"
    X-Header-Test = "b"

Deploying it will fail with:

When resolving config file /..../netlify.toml:
Could not parse configuration file
Cannot redefine existing key 'headers,values.X-Header-Test'.

However, if I update that to:

[[headers]]
  for = "/headers/test.json"
  [headers.values]
    X-Header-Test = "a"
    X-Header-test = "b"

It will parse correctly.

I believe the second version should also fail to parse, since the header names are case insensitive.

Configuration

  netlify-cli/3.8.4 linux-x64 node-v10.22.0

Expected behavior

I believe both versions should be identified as invalid.

[ehmicky]

Thanks for reporting this @mheffner.

This turned out to be a problem more subtle than it seemed, sorry for the long response!

Same header, same case

TOML

Per the TOML specification:

Defining a key multiple times is invalid.

As a consequence, the TOML parser we use fails at build time when the same property is defined twice.

HTTP

This behavior contrasts with HTTP headers which are semantically concatenated when specified multiple times in a given request. Some users might expect to be able to repeat a cache-control header as they would in HTTP.

Workarounds

Therefore, we currently offer the following workarounds in netlify.toml: using a multiline string (documented) or an array (undocumented, it seems):

[[headers]]
  for = "/path"
  [headers.values]
  example = '''
  one,
  two
  '''

[[headers]]
  for = "/path"
  [headers.values]
  example = ["one", "two"]

Those are handled the same as:

[[headers]]
  for = "/path"
  [headers.values]
  example = "one, two"

Different header sections

Additionally, it is currently possible to define the same header with the same case and the same path when using a new [[headers]] item:

[[headers]]
  for = "/path"
  [headers.values]
  example = "one"

[[headers]]
  for = "/path"
  [headers.values]
  example = "two"

In that case, the second header overrides the first one (as opposed to concatenating to it). I think this is intentional as it allows more specific rules to completely override earlier rules.

_headers file

We allow repetitions inside the _headers file. For example, the following:

/path
  example: one
  example: two

Will behave the same as:

/path
  example: one, two

This matches the HTTP behavior.

Same header, different case

HTTP

HTTP headers are case-insensitive, so some users might expect "same header, different case" to match the "same header, same case" behavior, as you mention @mheffner.

TOML

Since TOML is case-sensitive, keys with different cases are considered different keys. Therefore, our builds currently allow it.

Runtime

The runtime behavior of the following netlify.toml:

[[headers]]
  for = "/path"
  [headers.values]
  example = "one"
  Example = "two"

and _headers:

/path
  example: one
  Example: two

is: only Example: two is served. The second header overrides the first one (as opposed to concatenating to it).

This is quite different from repeating the same header but with the same case (which concatenates), which is quite unexpected considering HTTP is case-insensitive. IMHO This is most likely not what the user intended. Instead, they probably intend this to behave as concatenating the values.

Backward compatibility

If we were to forbid headers with the same name but different cases at build time, this would be a breaking change for sites which might depend on it at the moment, since this currently works at runtime.

Possible solutions

Solution A

Forbidding that pattern at build time would be a good way to prevent users from mistakenly think the header values will be concatenated. However, this change might break some sites.

Solution B

We could fix the behavior so that multiple headers with different cases are concatenated (not overridden) at runtime.

We would then keep failing at build time when parsing multiple headers with the same case in netlify.toml. We would consider it a TOML limitation (preventing us from completely mimicking the HTTP headers syntax) and offer the following workarounds: using an array or a multiline string. In other words, we would keep the current behavior there.

---

I am leaning towards solution B being the real fix for this problem. What are your thoughts on this @mheffner?

[mheffner]

Hey @ehmicky, thanks for the detailed response!

If we chose solution B, is that going to be a potential breaking change because it changes the behavior of how their application is behaving today? I'd be worried that we change the behavior of the application in a way they don't realize.

If we wanted to take solution A, could we initially make it a loud deprecation warning first so that we give users time to fix it? We wouldn't need to break builds immediately, but warn and then measure how many builds are actually using multi-case headers.

I would wonder on solution B if that feels too much like hacking around the TOML limitations. If our answer is array or multiline string, maybe better to be explicit in that requirement?

[ehmicky]

It does seem like both solutions would be breaking changes and require some initial warnings and metrics, as you mentioned.

I agree that both solutions are quite imperfect (including what you just mentioned), I am just not sure about alternative ones :thinking: Except for: keeping the current design and better documenting it?

[mheffner]

Yeah, I don't have a good idea on alternatives. Could we start by warning about these, likely rare, scenarios first? It does seem like mixing types is most likely a typo that a user did not intend and whose results are likely unexpected.

[ehmicky]

This sounds like a good first step! :+1:

[ehmicky]

PR to add the warning message: https://github.com/netlify/build/pull/3590

[ehmicky]

Humio query for the warning message.

Example of a site which does intend to override (current behavior), not concatenate headers of the same name (but different cases), between different paths.

To do: the warning message should include some information to know whether the two headers are using the same path or not. It seems to me the user intent (override vs concatenation) might differ depending on this.

[ehmicky]

PR to improve the warning message based on my comment above: https://github.com/netlify/build/pull/3594

New Humio queries:

• Query for headers with same name, different cases and same for path
• Query for headers with same name, different cases and different for path

It seems like almost all (95%) of sites are in the second category.

[ehmicky]

Findings

Findings from production monitoring:

• Only a small handful of sites are using headers with the same name, different cases and the same for path. It appears that most sites, but not all, intend the values of those headers to be concatenated.
• There are <200 sites which are using headers with the same name, different cases and different for paths. 99% of them are for the Cache-Control header. It appears that almost all sites intend the values of those headers to be overridden.

Note: this monitoring unfortunately only includes headers set in netlify.toml, not in _headers.

Problem

From those findings, the current behavior seems to be correct for headers specified twice in different sections: the second value overrides the first one. This applies regardless of whether the case differs and whether this is set in _headers or netlify.toml.

What could be improved is the behavior for headers specified twice in the same section. At the moment:

• If the case is the same, in _headers: the header value is concatenated
• If the case is the same, in netlify.toml: the build fails with a user error
• If the case differs, in _headers or netlify.toml: the header value is overridden

It seems like most users would intend:

• To concatenate the value in all of those cases.
• For the behavior to be consistent regardless of whether:
  • the case differs or not (like HTTP)
  • the header was specified in _headers or netlify.toml

However, fixing this inconsistency is difficult because:

• TOML does not allow the same key to be specified twice, i.e. using the same header with the same case in netlify.toml will always make TOML parsing fail.
• Any change of behavior will break backward compatibility.

Solution

I think the best way to avoid any ambiguity would be to forbid the same header to be specified twice in the same section. Either a multiline-string or a comma-separated list should be used instead.

However, this would definitely break some sites.The monitoring showed that the number of sites is fairly low for netlify.toml headers. However, this monitoring does not cover the _headers file.

I would suggest the two following potential solutions:

1. Print a deprecation warning for several weeks, then make this behavior a build error.
2. Keep the current behavior but print warning messages.

What do you think?

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in 1 year. It will be closed in 14 days if no further activity occurs. Thanks!

[github-actions[bot]]

This issue was closed because it had no activity for over 1 year.