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

IsValidShopDomainAsync should check if X-ShopId header has a value #995

Closed waheedbhatti closed 7 months ago

waheedbhatti commented 7 months ago

The current code checks if the X-ShopId header is present, but in some cases the header is present even for invalid URLs. However, even when the X-ShopId header is present there will never be a value for invalid URLs.

I suggest the following code be added to check if X-ShopId header as a value.

var headerValues = response.Headers.FirstOrDefault(h => h.Key.Equals("X-ShopId", StringComparison.OrdinalIgnoreCase)).Value;
if (headerValues != null && headerValues.Any(v => !string.IsNullOrWhiteSpace(v)))
{
    return true;
}
return false;
nozzlegear commented 7 months ago

Thanks for the report @waheedbhatti! I don't see any issue with the code you've provided and I'd be happy to fix the method. Just so I understand the problem clearly though, this is the code for IsValidShopDomainAsync right now:

var response = await client.SendAsync(msg, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
var hasShopIdHeader = response.Headers.Any(h => h.Key.Equals("X-ShopId", StringComparison.OrdinalIgnoreCase));

return hasShopIdHeader;

Are you saying this current code could incorrectly return true for invalid URLs? Is that because the invalid, non-Shopify URLs might also have their own X-ShopId header, or is there some other issue at play?

waheedbhatti commented 7 months ago

Yes, that's correct. The current code has incorrectly returned true for domains that are invalid. I'm not sure but what I think happens is that if a shop ever existed at the domain but no longer does, Shopify returns the X-ShopId header but with a null (empty) value.

Waheed Bhatti


From: Joshua Harms @.> Sent: Wednesday, January 17, 2024 12:18:23 PM To: nozzlegear/ShopifySharp @.> Cc: Waheed Bhatti @.>; Mention @.> Subject: Re: [nozzlegear/ShopifySharp] IsValidShopDomainAsync should check if X-ShopId header has a value (Issue #995)

Thanks for the report @waheedbhattihttps://github.com/waheedbhatti! I don't see any issue with the code you've provided and I'd be happy to fix the method. Just so I understand the problem clearly though, this is the code for IsValidShopDomainAsync right now:

var response = await client.SendAsync(msg, HttpCompletionOption.ResponseHeadersRead, cancellationToken); var hasShopIdHeader = response.Headers.Any(h => h.Key.Equals("X-ShopId", StringComparison.OrdinalIgnoreCase));

return hasShopIdHeader;

Are you saying this current code could incorrectly return true for invalid URLs? Is that because the invalid, non-Shopify URLs might also have their own X-ShopId header, or is there some other issue at play?

— Reply to this email directly, view it on GitHubhttps://github.com/nozzlegear/ShopifySharp/issues/995#issuecomment-1896625756, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AABIZ3VFWGALXJC5FUF32BTYPAWY7AVCNFSM6AAAAABB7BHDEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJWGYZDKNZVGY. You are receiving this because you were mentioned.Message ID: @.***>

--

The contents of this email message and any attachments are intended solely for the addressee(s) and may contain confidential and/or privileged information and may be legally protected from disclosure. If you are not the intended recipient of this message or their agent, or if this message has been addressed to you in error, please immediately alert the sender by reply email and then delete this message and any attachments. If you are not the intended recipient, you are hereby notified that any use, dissemination, copying, or storage of this message or its attachments is strictly prohibited. 

nozzlegear commented 7 months ago

That makes sense to me. I'm working on a release for today actually, so I'll make sure this gets fixed and published at the same time!

waheedbhatti commented 7 months ago

Thank you.

nozzlegear commented 7 months ago

Should be fixed in 6.12.0 on Nuget!