nozzlegear / ShopifySharp

ShopifySharp is a .NET library that helps developers easily authenticate with and manage Shopify stores.
https://nozzlegear.com/shopify-development-handbook
MIT License
742 stars 308 forks source link

CloneableRequestMessage can fail to clone content and then throw an ObjectDisposedException #1001

Closed nozzlegear closed 7 months ago

nozzlegear commented 7 months ago

The CloneableRequestMessage class performs this check when it clones itself:

HttpContent newContent = Content;

if (newContent != null && newContent is JsonContent c)
{
    newContent = c.Clone();
}

This will only clone JsonContent, which works in almost all cases except when it doesn't. For example, the PartnerService is using StringContent. I believe the GraphService also uses StringContent.

If the CloneableRequestMessage has any kind of content in it that is not specifically JsonContent, and the message gets cloned and reused, an ObjectDisposedException will be thrown when the next request is sent. This seems to be because the original request message is disposed, which then tries to dispose the HttpContent that was attached to it.

To fix this, I want to rework the whole concept of the CloneableRequestMessage. Rather than cloning the request message, we'll use a sort of HttpMessage object that has configuration data about the request but is not disposable and does not do the request itself. It's simply used to create the request. It will hold the data being sent to Shopify, along with the Http method used, the url, headers, and so on.