Azure / Azure/azure-functions-dotnet-worker

`DefaultFunctionInputBindingFeature` ignores input conversion failures under cetrain conditions.

Open
#2,978 4 comments 0 reactions 1 assignee Claimed by @satvu View on GitHub
enhancement needs-discussion
Dominant language
C#
Stars
466
Forks
215
Avg merge
3d 10h
Merged PRs (30d)
7

Description

### Description

While debugging an issue in the service, I discovered that this code block in `DefaultFunctionInputBindingFeature` leads to an unwanted behavior:
https://github.com/Azure/azure-functions-dotnet-worker/blob/61d3c41dcc6aa3918db20fa5d8faa59e147b08d6/src/DotNetWorker.Core/Context/Features/DefaultFunctionInputBindingFeature.cs#L64-L70

It is part of large `if-else` block that checks conversion results. The problem here is that it requires `source is not null`. If `source` is `null`, then `CoversionResult.Failed` is never treated and is silently dropped. As a result, instead of some hard failure, the function input is set to `null`, which produces a null-ref exception down stream in user code.

The only way to fix the problem that I found was to explicitly throw an exception from converter instead of returning `ConversionResult.Failed`, which is technically incorrect.

Could somebody please explain what is the reason for the `source is not null` check here or why there is no branch for when conversion fails but `source is null`?

### Steps to reproduce

We have a custom type that we populate from request headers. We crated an implementation of `IInputConverter` that handles this conversion, and a dedicated attribute
```csharp
[AttributeUsage(AttributeTargets.Parameter)]
public sealed class CustomContextAttribute() : InputConverterAttribute(typeof(CustomContextConverter));
```

Then we use it like so:

```csharp
Task SomeMethod(
[HttpTrigger] HttpRequestData request,
[CustomContext] CustomContextType customContext,
// blah blah
) => null;
```

**Expected behavior** : if `customContext` converter returns a failure, an `FunctionInputConverterException` is thrown from here
https://github.com/Azure/azure-functions-dotnet-worker/blob/61d3c41dcc6aa3918db20fa5d8faa59e147b08d6/src/DotNetWorker.Core/Context/Features/DefaultFunctionInputBindingFeature.cs#L94-L99

**Actual behavior**: Conversion failure is silently ignored, `customContext` is initialized to `null`, which breaks user code.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.