dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.4k stars 10k forks source link

Verification of Patch Request (The following request in image gave 500 error which can be prevented but do not) #38540

Open abdollahkahne opened 2 years ago

abdollahkahne commented 2 years ago

I am not sure which Library is responsible for this issue and I am sorry if it is not related to this repo. I am trying to implement a Patch Request API using Asp.Net Core API 5.0 and so I have a parameter in my Controller Action which Takes JsonPatchDocument<T>. As I tried it I noticed that we should have three stage of verification for this model including: 0- Validation of JsonPatchDocument according to Rest Standard (for example it should be an array! its element have standard shape including op and path and other constraints). This Validation should done after binding model and it do not happen now 1- Validation of JsonPatchDocument according to our Entity Type (For example does selected path existed in our entity) 2- Validation of Resulted T model from applying requested patch (for example field is email type and other verification attributes added to model T.

From the above list the validations 1 and 2 happens and is okay but validation 0 does not handle which I think it should be an step right after model binding. For example when sending the following patch request I get 500 status code error. image The action responsible for this code is as below:

`

    [HttpPatch("{id}")]
    public IActionResult PartiallyUpdateEmployee(Guid companyId, [FromRoute(Name = "id")] Guid employeeId, JsonPatchDocument<EmployeeForUpdateDto> input)
    {
        if (input == null)
        {
            _logger.LogError($"The Patch body is null for request {HttpContext.TraceIdentifier}");
            return BadRequest("Patch reuest body is null");
        }
        if (ModelState.IsValid) // *******This is where Validation 0 should  prevent the code from continuing and not**************
        {
            // check company is not null
            var company = _repository.Company.GetById(companyId, false);
            if (company == null)
            {
                _logger.LogInfo($"{HttpContext.TraceIdentifier}: the company with Id {companyId} does not exist in database");
                return NotFound();
            }

            var employee = _repository.Employee.GetEmployee(companyId, employeeId, true);
            if (employee == null)
            {
                _logger.LogInfo($"{HttpContext.TraceIdentifier}: the employee with Id {employeeId} for Company with Id {companyId} does not exist in database");
                return NotFound();
            }

            var entityPart = _mapper.Map<EmployeeForUpdateDto>(employee);

            // In the following we have two (plus one!) type of validation

            input.ApplyTo(entityPart, ModelState); // Verification stage 1 which works as expected
            TryValidateModel(entityPart); // Verification stage 2 which works as expected
            // since Model State changed we should recheck it here
            if (ModelState.IsValid)
            {
                _mapper.Map(entityPart, employee); 
                _repository.Save();
                //return changed employee to user
                var output = _mapper.Map<EmployeeDto>(employee);
                return Ok(output);
            }

        }
        return BadRequest(ModelState);
    }`

` I have added the following library for handling JsonPatchDocument Microsoft.AspNetCore.JsonPatch` And I appreciate if checking the request against REST Standard for patch request done some where to prevent 500 status code error. My Response image My logs: image and the related repository to test this: https://github.com/CodeMazeBlog/httpclient-aspnetcore/tree/main/starter/CompanyEmployees

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.