OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
454 stars 159 forks source link

Batch request fails with encoded URL #1009

Open rahul7720 opened 1 year ago

rahul7720 commented 1 year ago

Assemblies affected ASP.NET Core OData 8.x

Describe the bug When using multipart/mixed batch requests, with encoded URL in the batch request body (i.e., ' is encoded to %27). However, this causes an Internal Server Error (HTTP 500) in ASP.NET Core 7 OData, which seems unable to correctly decode these URLs in the batch request body.

Reproduce steps

Request/Response Request

--batch_c3f9057d-2492-4a77-b2a6-099d7c7adfe9
Content-Type: multipart/mixed; boundary=changeset_ae1277da-d1d0-473e-a0ae-b982bdaeb1fc

--changeset_ae1277da-d1d0-473e-a0ae-b982bdaeb1fc
Content-Type: application/http
Content-Transfer-Encoding: binary
Content-ID: 1

GET https://localhost:44377/odata/Products(%27someid%27) HTTP/1.1
If-Match: *

--changeset_ae1277da-d1d0-473e-a0ae-b982bdaeb1fc--
--batch_c3f9057d-2492-4a77-b2a6-099d7c7adfe9--

Response

--batchresponse_9cb15c52-d513-4f22-bfda-66c7ca50a75b
Content-Type: application/http
Content-Transfer-Encoding: binary

HTTP/1.1 500 Internal Server Error

--batchresponse_9cb15c52-d513-4f22-bfda-66c7ca50a75b--

Expected behavior

Additional context It seems the URL encoding issue only arises when special characters are present in the URLs within the batch request body. Single requests work as expected, and batch requests without special characters in the URLs also work as expected.

rahul7720 commented 1 year ago

I resolved this issue with a temporary workaround using a custom batch handler. But the issue needs to be fixed in the core library.

public class CustomODataBatchHandler : DefaultODataBatchHandler
{
    public async override Task<IList<ODataBatchRequestItem>> ParseBatchRequestsAsync(HttpContext context)
    {
        var requests = await base.ParseBatchRequestsAsync(context);
        try
        {
            foreach (var requestItem in requests)
            {
                if (requestItem is OperationRequestItem)
                {
                    var httpRequest = ((OperationRequestItem)requestItem).Context.Request;
                    httpRequest.Path = Uri.UnescapeDataString(httpRequest.Path);
                }
            }
        }
        catch (Exception ex)
        {
            //handle exceptions
        }        
        return requests;
    }
}
corranrogue9 commented 1 year ago

We should update our logic to start parsing the URLs in the batch request body at the same layer that we start decoding the URLs so that the behavior is consistent for batch requests.

According to the standard, URLs should be percent encoded in batch requests:

URLs must be correctly percent-encoded. For relative URLs this means that colons in the path part, especially within key values, MUST be percent-encoded to avoid confusion with the scheme separator. Colons within the query part, i.e. after the question mark character (?), need not be percent-encoded.

It seems like there might be some related ODL issue, I will create an issue for that if it's discovered during the investigation.

rahul7720 commented 1 year ago

We should update our logic to start parsing the URLs in the batch request body at the same layer that we start decoding the URLs so that the behavior is consistent for batch requests.

According to the standard, URLs should be percent encoded in batch requests:

URLs must be correctly percent-encoded. For relative URLs this means that colons in the path part, especially within key values, MUST be percent-encoded to avoid confusion with the scheme separator. Colons within the query part, i.e. after the question mark character (?), need not be percent-encoded.

It seems like there might be some related ODL issue, I will create an issue for that if it's discovered during the investigation.

The encoded URLs are generated by the odata.net library. May be this issue can be related: https://github.com/OData/odata.net/issues/2292