From 773073e007e718566f2e214761c107aa2759f8e3 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 31 May 2017 06:52:55 -0500 Subject: [PATCH 1/3] fix(type-helper): do not silently return default something unexpected happened, we should return an error --- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 7 ++----- test/UnitTests/Internal/TypeHelper_Tests.cs | 11 +++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index 6ceb3b06ac..60574ff295 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -24,12 +24,9 @@ public static object ConvertType(object value, Type type) return Convert.ChangeType(stringValue, type); } - catch (Exception) + catch (Exception e) { - if (type.GetTypeInfo().IsValueType) - return Activator.CreateInstance(type); - - return null; + throw new FormatException($"{ value } cannot be converted to { type.GetTypeInfo().Name }", e); } } } diff --git a/test/UnitTests/Internal/TypeHelper_Tests.cs b/test/UnitTests/Internal/TypeHelper_Tests.cs index 912e516af9..7f05edd892 100644 --- a/test/UnitTests/Internal/TypeHelper_Tests.cs +++ b/test/UnitTests/Internal/TypeHelper_Tests.cs @@ -19,5 +19,16 @@ public void Can_Convert_DateTimeOffsets() // assert Assert.Equal(dto, result); } + + [Fact] + public void Bad_DateTimeOffset_String_Throws() + { + // arrange + var formattedString = "this_is_not_a_valid_dto"; + + // act + // assert + Assert.Throws(() => TypeHelper.ConvertType(formattedString, typeof(DateTimeOffset))); + } } } From 734bd3e06915520389df9680b062c6c5bbab2f22 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 31 May 2017 06:53:11 -0500 Subject: [PATCH 2/3] fix(de-serialization): improve error handling --- .../Formatters/JsonApiReader.cs | 9 +++ .../Serialization/JsonApiDeSerializer.cs | 79 ++++++++++++------- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs index f9d38c2cfb..103c1b9ab5 100644 --- a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs +++ b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs @@ -1,6 +1,8 @@ using System; using System.IO; +using System.Text; using System.Threading.Tasks; +using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Serialization; using JsonApiDotNetCore.Services; using Microsoft.AspNetCore.Mvc.Formatters; @@ -50,6 +52,13 @@ public Task ReadAsync(InputFormatterContext context) context.HttpContext.Response.StatusCode = 422; return InputFormatterResult.FailureAsync(); } + catch(JsonApiException jex) + { + _logger?.LogError(new EventId(), jex, "An error occurred while de-serializing the payload"); + context.HttpContext.Response.StatusCode = jex.GetStatusCode(); + context.HttpContext.Response.Body = new MemoryStream(Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(jex.GetError()))); + return InputFormatterResult.FailureAsync(); + } } private string GetRequestBody(Stream body) diff --git a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs index d64d18366f..f9cb803c27 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs @@ -16,7 +16,7 @@ public class JsonApiDeSerializer : IJsonApiDeSerializer private readonly IJsonApiContext _jsonApiContext; private readonly IGenericProcessorFactory _genericProcessorFactor; - public JsonApiDeSerializer( + public JsonApiDeSerializer( IJsonApiContext jsonApiContext, IGenericProcessorFactory genericProcessorFactory) { @@ -26,9 +26,16 @@ public JsonApiDeSerializer( public object Deserialize(string requestBody) { - var document = JsonConvert.DeserializeObject(requestBody); - var entity = DocumentToObject(document.Data); - return entity; + try + { + var document = JsonConvert.DeserializeObject(requestBody); + var entity = DocumentToObject(document.Data); + return entity; + } + catch (Exception e) + { + throw new JsonApiException("400", "Failed to deserialize request body", e.Message); + } } public object Deserialize(string requestBody) @@ -38,26 +45,40 @@ public object Deserialize(string requestBody) public object DeserializeRelationship(string requestBody) { - var data = JToken.Parse(requestBody)["data"]; + try + { + var data = JToken.Parse(requestBody)["data"]; - if(data is JArray) - return data.ToObject>(); + if (data is JArray) + return data.ToObject>(); - return new List { data.ToObject() }; + return new List { data.ToObject() }; + } + catch (Exception e) + { + throw new JsonApiException("400", "Failed to deserialize request body", e.Message); + } } public List DeserializeList(string requestBody) { - var documents = JsonConvert.DeserializeObject(requestBody); + try + { + var documents = JsonConvert.DeserializeObject(requestBody); + + var deserializedList = new List(); + foreach (var data in documents.Data) + { + var entity = DocumentToObject(data); + deserializedList.Add((TEntity)entity); + } - var deserializedList = new List(); - foreach (var data in documents.Data) + return deserializedList; + } + catch (Exception e) { - var entity = DocumentToObject(data); - deserializedList.Add((TEntity)entity); + throw new JsonApiException("400", "Failed to deserialize request body", e.Message); } - - return deserializedList; } private object DocumentToObject(DocumentData data) @@ -66,7 +87,7 @@ private object DocumentToObject(DocumentData data) _jsonApiContext.RequestEntity = contextEntity; var entity = Activator.CreateInstance(contextEntity.EntityType); - + entity = SetEntityAttributes(entity, contextEntity, data.Attributes); entity = SetRelationships(entity, contextEntity, data.Relationships); @@ -106,8 +127,8 @@ private object SetEntityAttributes( } private object SetRelationships( - object entity, - ContextEntity contextEntity, + object entity, + ContextEntity contextEntity, Dictionary relationships) { if (relationships == null || relationships.Count == 0) @@ -117,18 +138,18 @@ private object SetRelationships( foreach (var attr in contextEntity.Relationships) { - entity = attr.IsHasOne - ? SetHasOneRelationship(entity, entityProperties, attr, contextEntity, relationships) + entity = attr.IsHasOne + ? SetHasOneRelationship(entity, entityProperties, attr, contextEntity, relationships) : SetHasManyRelationship(entity, entityProperties, attr, contextEntity, relationships); } return entity; } - private object SetHasOneRelationship(object entity, - PropertyInfo[] entityProperties, - RelationshipAttribute attr, - ContextEntity contextEntity, + private object SetHasOneRelationship(object entity, + PropertyInfo[] entityProperties, + RelationshipAttribute attr, + ContextEntity contextEntity, Dictionary relationships) { var entityProperty = entityProperties.FirstOrDefault(p => p.Name == $"{attr.InternalRelationshipName}Id"); @@ -142,7 +163,7 @@ private object SetHasOneRelationship(object entity, { var relationshipAttr = _jsonApiContext.RequestEntity.Relationships .SingleOrDefault(r => r.PublicRelationshipName == relationshipName); - + var data = (Dictionary)relationshipData.ExposedData; if (data == null) return entity; @@ -159,9 +180,9 @@ private object SetHasOneRelationship(object entity, } private object SetHasManyRelationship(object entity, - PropertyInfo[] entityProperties, - RelationshipAttribute attr, - ContextEntity contextEntity, + PropertyInfo[] entityProperties, + RelationshipAttribute attr, + ContextEntity contextEntity, Dictionary relationships) { var entityProperty = entityProperties.FirstOrDefault(p => p.Name == attr.InternalRelationshipName); @@ -179,7 +200,7 @@ private object SetHasManyRelationship(object entity, var genericProcessor = _genericProcessorFactor.GetProcessor(attr.Type); var ids = relationshipData.ManyData.Select(r => r["id"]); - genericProcessor.SetRelationships(entity, attr, ids); + genericProcessor.SetRelationships(entity, attr, ids); } return entity; From 2b650ab28ec3c62862c32c826bf101533b4a4f7e Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 31 May 2017 06:56:38 -0500 Subject: [PATCH 3/3] chore(appveyor): dont skip symbols on deploy --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 55c43a7f82..ec135d19bb 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -43,14 +43,14 @@ deploy: server: https://www.myget.org/F/research-institute/api/v2/package api_key: secure: 6CeYcZ4Ze+57gxfeuHzqP6ldbUkPtF6pfpVM1Gw/K2jExFrAz763gNAQ++tiacq3 - skip_symbols: true + skip_symbols: false on: branch: develop - provider: NuGet server: https://www.myget.org/F/jadnc/api/v2/package api_key: secure: 6CeYcZ4Ze+57gxfeuHzqP6ldbUkPtF6pfpVM1Gw/K2jExFrAz763gNAQ++tiacq3 - skip_symbols: true + skip_symbols: false on: branch: unstable - provider: NuGet