Comments (8)
@myquay I wasn't able to accomplish the above (overriding ApplyUpdatesTo
) and after further research determined that modifying the method SetValueFromPath
with some custom logic would resolve this.
For further context, I was trying to return the enums that could be used if an end API user inputted an invalid value. For an API field that has a nullable enum type, if a user submitted an incorrect value (one not in the enum), he or she could get a response something like below (depending on how this package is integrated with your API responses):
"Failed to set value at path \"/my_field\" while performing \"replace\" operation: Error converting value \"my_bad_enum_value\" to type 'System.Nullable`1[path/directory]'. Path '', line 1, position 9."
This could be confusing and potentially a security issue (debatable of course) by exposing the internal directory. Or for those less tech savvy, it is just not super clear. Passing invalid values to the API should not normally happen though especially if you expose all of the valid values of the enum in your documentation.
With the changes I did, now SetValueFromPath
can potentially throw an exception something like this (which is more clear in my opinion):
Invalid value \"bad_enum_value\" for Foo. Possible values are: FirstEnum,SecondEnum,ThirdEnum
@myquay I am not sure if others would like this change or if it makes sense for the direction of this repo. Let me know what you think. I'd be happy to open a pull request if anything.
from jsonpatch.
@myquay Thanks for reviewing both of my pull requests and merging them in. I am excited to use the new release!
from jsonpatch.
Hi @broderickt2,
Thanks for taking the time to write up the issue you're having.
The security issue
Internal exceptions should be managed via. the frameworks error handling features: https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis/handle-errors?view=aspnetcore-7.0 and not exposed directly to the end user - that's outside the scope of this library.
The error message
In regard to the actual error message returned from the library it's hard to get a message that's suitable for all applications and it isn't localised etc - so I think tinkering with that that isn't the correct path to take.
However, that's not to say it can't be improved. I think a good approach to that is to return additional contextual information in the exception thrown so the error handler in a specific API implementation has all the information required to generate a suitable error message for its particular use case.
Just off the top of my head something like:
public class JsonPatchOperationException : JsonPatchException {
/// Additional properties that can be used to generate a message like
/// 'Invalid value \"bad_enum_value\" for Foo. Possible values are: FirstEnum,SecondEnum,ThirdEnum'.
/// E.g., OperationType, Path, Target Type, Supplied Value etc...
}
Which can be thrown in place of the base JsonPatchException
in the SetValueFromPath
method.
Then the downstream API error handler can catch errors of this type and has the required information from the library to generate a useful error message without baking a particular message into the library itself.
Let me know how that sounds and if it covers the use case you've described. I'd be more than happy to work with you on getting the change through - take a stab at an initial implementation and open a PR and we can go from there.
from jsonpatch.
@myquay Thanks for the prompt response and explaining a lot of extra information. I did some research on my end and you are correct in that I should manage the exception on my end and not rely on this library to return custom exception messages.
I think improving the exceptions that are returned from this library is a great idea. I got some ideas implemented on my local machine but cannot create a branch to push to. Do I need to be a collaborator to create a branch and pull request? Or are you able to create a branch I can push to? I thought about forking the repo and then creating a pull request upstream but don't think this will work from what I am reading here due to my access.
from jsonpatch.
What you read there is the process, fork the repository and when you're happy with the changes submit a pull request 😊
from jsonpatch.
@myquay That worked! I got a pull request here when you get a moment.
from jsonpatch.
@myquay Thanks for merging in the pull request. Left one last question here for you.
from jsonpatch.
@broderickt2 Thanks for the contribution and taking the time to clean up those namespaces.
from jsonpatch.
Related Issues (20)
- Support for IDictionary HOT 5
- Update NuGet package version HOT 3
- Issue replacing an List<T> property HOT 1
- NuSpec file has different dependency than csproj HOT 1
- Support Dictionaries HOT 3
- DTO example not working? HOT 3
- repository.Save(objectToUpdate); HOT 1
- Unable to patch an Enum based on description HOT 6
- Support Swagger/Swashbuckle HOT 1
- Position larger than array size
- Patch operation replace does not support value replacement of a property in a an element in an array
- Remove new() constraint for TEntity generic parameter of JsonPatchDocument
- Support ASP.NET Core Controllers
- Release date for V2 HOT 2
- ModelState.IsValid Not Returning Accurate Results HOT 2
- Enum Parsing Case Sensitivity HOT 2
- Parsing DateTimeOffset HOT 1
- IDictionary properties not handled as dictionaries HOT 1
- GetProperty is internal so seems like no way to create custom resolver HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from jsonpatch.