IHeaderDictionary.Append not adding headers on empty values

[eiriktsarpalis]

Describe the bug

This is probably by design, but it does lead to surprising behaviour

To Reproduce

Steps to reproduce the behavior:

1. Using ASP.NET Core 3.0.0-preview9.19412.6
2. Run

   
   var headers = new Microsoft.AspNetCore.Http.HeaderDictionary();

headers.Add("header1", ""); headers.Append("header2", ""); headers.Append("header3", new string[] { "", "" });

headers.ContainsKey("header1"); // true headers.ContainsKey("header2"); // false headers.ContainsKey("header3"); // true

When I wire the above to a kestrel app:

$ curl -ki https://127.0.0.1:5001/headers HTTP/2 200 date: Tue, 13 Aug 2019 08:14:13 GMT server: Kestrel header1: header3: header3:

### Expected behavior

`"header2"` should be populated with a `StringValue` instance containing a single empty string. This expectation comes from the behaviour `HeaderDictionary.Add` and the fact that asp.net core is supporting empty header values. Also, appending empty values is not idempotent in general, e.g.
```csharp
var headers = new Microsoft.AspNetCore.Http.HeaderDictionary();

headers.Add("header1", "");
headers.Append("header1", "");

The behaviour seems to be caused by this line.

[Tratcher]

HTTP headers with empty values have no semantic meaning in the spec (except Host) and you'll get widely varying behavior trying to use them. Do you have an actual scenario for this?

Many servers will filter out the values. It's actually a little odd Kestrel doesn't.

[eiriktsarpalis]

Do you have an actual scenario for this?

Not quite. Just stress testing with random headers. I added empty header values since kestrel seemed to support them. However once I moved to trailing headers I started seeing failures as HttpResponse.AppendTrailer() builds on top of the IHeaderDictionary.Append() method.

[thomaslevesque]

Do you have an actual scenario for this?

I do. WOPI expects the X-WOPI-Lock header to be set to an empty string in some situations. If it's not there, the validation tests don't pass (see the documentation)

I have a WOPI host implementation that worked fine in ASP.NET Core 2.1, but after migrating to ASP.NET Core 3.1, it no longer returns that header when the value is empty.

Note: the problem only occurs when the app is deployed in Azure. When I test locally with Kestrel or IIS Express, it works fine. So I suspect the problem might not be in ASP.NET Core itself, but maybe somewhere else... AspNetCoreModule, maybe?

[thomaslevesque]

I think my problem is related to this: https://github.com/dotnet/aspnetcore/pull/12486 The only workaround that I found is to set the header to a string with only whitespace. The value is actually trimmed before being sent to the client, but it prevents the header from being removed completely. It seems pretty brittle, though...

[Tratcher]

https://wopi.readthedocs.io/projects/wopirest/en/latest/files/Lock.html?highlight=x-wopi-lock

the X-WOPI-Lock response header should be set to the empty string or omitted completely.

Even that spec says an empty header is equivalent to not adding it at all.

[thomaslevesque]

@Tratcher indeed, I didn't notice that bit. But the spec contradicts itself, because later it says:

X-WOPI-Lock – A string value identifying the current lock on the file. This header must always be included when responding to the request with 409 Conflict. It should not be included when responding to the request with 200 OK.

In any case, the validation tests require that the header is present, even if it's empty. Omitting it make the tests fail. So I guess they should either fix their doc, or fix their tests...

[Tratcher]

Its certainly worth getting clarification from them about it. The docs seem correct to treat a missing and empty header the same. If they give you trouble send them to me.

[thomaslevesque]

@Tratcher I will, thanks!