OData Batch Requests fails to handle duplicate http attributes

[apiaskowski]

Sending OData batch requests with duplicate http headers will cause an System.ArgumentException that indicates that duplicate http headers are not validated correct.

Assemblies affected

Microsoft.OData.Core 7.15.0

Reproduce steps

Create a basic MVC OData service and enable batch requests.
Submit any batch request, which includes the same http header twice

Expected result

If a duplicate http header has been send, the duplicate header(s) should be ignored or overwrite the previous value.

Actual result

The OData batch requests returns an HTTP 500 error, with the System.ArgumentException: An item with the same key has already been added. Key: .
This is caused by the Microsoft.OData.JsonLight.BatchPayloadItemPropertiesCache which doesn't validate, if an http header already has been added to the headers Dictionary.

[habbes]

@apiaskowski thanks for reporting this. Would you consider contributing a PR to address this?

[apiaskowski]

@habbes my PR bugfix is in preparation.

[apiaskowski]

Dear @habbes,

I've got to take back my PR, because of legal reasons with the CLA that have to be sorted out by my employer.
Do you mind, if I suggest you a potential fix, which I've implemented & tested locally?

The issue occurs, because of missing checks in the Microsoft.OData.JsonLight.ODataJsonLightBatchPayloadItemPropertiesCache class, in specific within the method ScanJsonProperties and ScanJsonPropertiesAsync.
All instances of jsonProperties.Add() and headers.Add() should check, if an element already exists in the respective Dictionary.
Depending on the desired handling, either skip duplicate elements or overwrite the element's value (don't forget to read the next Property by the asynchronousJsonReader, if an element is skipped).

Examples:

• jsonProperties.Add(propertyName, this.jsonReader.ReadStringValue());
• headers.Add(headerName, headerValue);

In addition I've observed potential ArgumentNullException to occur, when the asynchronousJsonReader.ReadPrimitiveValueAsync() returns an null value and afterwards is converted into a string value without check (only occurs for the Async implementation).

Examples:

• await this.asynchronousJsonReader.ReadStringValueAsync().ConfigureAwait(false)

[habbes]

@apiaskowski was this issue observed when using JSON batch or Multipart mixed format or both formats?

[apiaskowski]

@apiaskowski was this issue observed when using JSON batch or Multipart mixed format or both formats?

We've been using the content-type json and not multipart.
We don't have a scenario for batching with the multipart format.