Don't throw exception when converting values using binder or JsonConverter

This commit is contained in:
crobibero 2020-11-12 08:06:25 -07:00
parent 5669d1dad2
commit 0c23b8cadf
3 changed files with 89 additions and 36 deletions

View File

@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Logging;
namespace Jellyfin.Api.ModelBinders
{
@ -11,6 +13,17 @@ namespace Jellyfin.Api.ModelBinders
/// </summary>
public class CommaDelimitedArrayModelBinder : IModelBinder
{
private readonly ILogger<CommaDelimitedArrayModelBinder> _logger;
/// <summary>
/// Initializes a new instance of the <see cref="CommaDelimitedArrayModelBinder"/> class.
/// </summary>
/// <param name="logger">Instance of the <see cref="ILogger{CommaDelimitedArrayModelBinder}"/> interface.</param>
public CommaDelimitedArrayModelBinder(ILogger<CommaDelimitedArrayModelBinder> logger)
{
_logger = logger;
}
/// <inheritdoc/>
public Task BindModelAsync(ModelBindingContext bindingContext)
{
@ -20,16 +33,8 @@ namespace Jellyfin.Api.ModelBinders
if (valueProviderResult.Length > 1)
{
var result = Array.CreateInstance(elementType, valueProviderResult.Length);
for (int i = 0; i < valueProviderResult.Length; i++)
{
var value = converter.ConvertFromString(valueProviderResult.Values[i].Trim());
result.SetValue(value, i);
}
bindingContext.Result = ModelBindingResult.Success(result);
var typedValues = GetParsedResult(valueProviderResult.Values, elementType, converter);
bindingContext.Result = ModelBindingResult.Success(typedValues);
}
else
{
@ -37,13 +42,8 @@ namespace Jellyfin.Api.ModelBinders
if (value != null)
{
var values = Array.ConvertAll(
value.Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries),
x => converter.ConvertFromString(x?.Trim()));
var typedValues = Array.CreateInstance(elementType, values.Length);
values.CopyTo(typedValues, 0);
var splitValues = value.Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries);
var typedValues = GetParsedResult(splitValues, elementType, converter);
bindingContext.Result = ModelBindingResult.Success(typedValues);
}
else
@ -55,5 +55,36 @@ namespace Jellyfin.Api.ModelBinders
return Task.CompletedTask;
}
private Array GetParsedResult(IReadOnlyList<string> values, Type elementType, TypeConverter converter)
{
var parsedValues = new object?[values.Count];
var convertedCount = 0;
for (var i = 0; i < values.Count; i++)
{
try
{
parsedValues[i] = converter.ConvertFromString(values[i]?.Trim());
convertedCount++;
}
catch (FormatException e)
{
_logger.LogWarning(e, "Error converting value.");
}
}
var typedValues = Array.CreateInstance(elementType, convertedCount);
var typedValueIndex = 0;
for (var i = 0; i < parsedValues.Length; i++)
{
if (parsedValues[i] != null)
{
typedValues.SetValue(parsedValues[i], typedValueIndex);
typedValueIndex++;
}
}
return typedValues;
}
}
}

View File

@ -32,13 +32,34 @@ namespace MediaBrowser.Common.Json.Converters
return Array.Empty<T>();
}
var entries = new T[stringEntries.Length];
var parsedValues = new object[stringEntries.Length];
var convertedCount = 0;
for (var i = 0; i < stringEntries.Length; i++)
{
entries[i] = (T)_typeConverter.ConvertFrom(stringEntries[i].Trim());
try
{
parsedValues[i] = _typeConverter.ConvertFrom(stringEntries[i].Trim());
convertedCount++;
}
catch (FormatException)
{
// TODO log when upgraded to .Net5
// _logger.LogWarning(e, "Error converting value.");
}
}
return entries;
var typedValues = new T[convertedCount];
var typedValueIndex = 0;
for (var i = 0; i < stringEntries.Length; i++)
{
if (parsedValues[i] != null)
{
typedValues.SetValue(parsedValues[i], typedValueIndex);
typedValueIndex++;
}
}
return typedValues;
}
return JsonSerializer.Deserialize<T[]>(ref reader, options);

View File

@ -5,6 +5,7 @@ using System.Threading.Tasks;
using Jellyfin.Api.ModelBinders;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Primitives;
using Moq;
using Xunit;
@ -21,7 +22,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamString = "lol,xd";
var queryParamType = typeof(string[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }),
@ -46,7 +47,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamString = "42,0";
var queryParamType = typeof(int[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }),
@ -71,7 +72,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamString = "How,Much";
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }),
@ -96,7 +97,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamString = "How,,Much";
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }),
@ -122,7 +123,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamString2 = "Much";
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
@ -150,7 +151,7 @@ namespace Jellyfin.Api.Tests.ModelBinders
var queryParamValues = Array.Empty<TestType>();
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
@ -172,13 +173,13 @@ namespace Jellyfin.Api.Tests.ModelBinders
}
[Fact]
public async Task BindModelAsync_ThrowsIfCommaDelimitedEnumArrayQueryIsInvalid()
public async Task BindModelAsync_EnumArrayQuery_BindValidOnly()
{
var queryParamName = "test";
var queryParamString = "🔥,😢";
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }),
@ -189,20 +190,20 @@ namespace Jellyfin.Api.Tests.ModelBinders
bindingContextMock.Setup(b => b.ModelType).Returns(queryParamType);
bindingContextMock.SetupProperty(b => b.Result);
Func<Task> act = async () => await modelBinder.BindModelAsync(bindingContextMock.Object);
await Assert.ThrowsAsync<FormatException>(act);
await modelBinder.BindModelAsync(bindingContextMock.Object);
Assert.True(bindingContextMock.Object.Result.IsModelSet);
Assert.Empty((TestType[])bindingContextMock.Object.Result.Model);
}
[Fact]
public async Task BindModelAsync_ThrowsIfCommaDelimitedEnumArrayQueryIsInvalid2()
public async Task BindModelAsync_EnumArrayQuery_BindValidOnly_2()
{
var queryParamName = "test";
var queryParamString1 = "How";
var queryParamString2 = "😱";
var queryParamType = typeof(TestType[]);
var modelBinder = new CommaDelimitedArrayModelBinder();
var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>());
var valueProvider = new QueryStringValueProvider(
new BindingSource(string.Empty, string.Empty, false, false),
@ -217,9 +218,9 @@ namespace Jellyfin.Api.Tests.ModelBinders
bindingContextMock.Setup(b => b.ModelType).Returns(queryParamType);
bindingContextMock.SetupProperty(b => b.Result);
Func<Task> act = async () => await modelBinder.BindModelAsync(bindingContextMock.Object);
await Assert.ThrowsAsync<FormatException>(act);
await modelBinder.BindModelAsync(bindingContextMock.Object);
Assert.True(bindingContextMock.Object.Result.IsModelSet);
Assert.Single((TestType[])bindingContextMock.Object.Result.Model);
}
}
}