calcinai / xero-php

A php library for the Xero API, with a cleaner OAuth interface and ORM-like abstraction.
MIT License
360 stars 260 forks source link

\XeroPHP\Application\Query::execute() can fail to search due to + (plus) symbol #931

Open edegaudenzi opened 1 week ago

edegaudenzi commented 1 week ago

PHP version: x.y.z (hint: php --version) 8.3.6 (Ubuntu 24.04.1) calcinai/xero-php v2.7.0

Description Very long story-short: if I want to perform a search like where=Name=="Canal+ SARL", the actual executed query is where=Name%3D%3D%22Canal+%20SARL%22, returning NULL. '+' (plus) symbol didn't become %2B but instead remained + and Xero can't find it (Xero probably thinks + is rather a space character - the %20 to be explicit - ). Unfortunately this impacts literally every Xero entity: Contacts, Invoices, etc.

How to reproduce Of course you need to have a Contact in Xero called Canal+ SARL, then:

(new \XeroPHP\Application($access_token, $tenant_id))->load(\XeroPHP\Models\Accounting\Contact::class)->where('Name', 'Canal+ SARL')->execute()

It returns NULL instead of the existing Contact

Additional context I've spent quite a lot of time understanding that in this case Guzzle is the one to blame, but Guzzle is also right at the same time; as you imagine it's a long and complicated story. I've also opened an issue here if you want to see the evil in the details. On the xero-php package side, one possible solution to prevent this from happening is replacing + into %2B by ourselves. With this change: where=Name=="Canal%2B SARL" becomes where=Name%3D%3D%22Canal%2B%20SARL%22 and will find the Contact.

calcinai commented 4 days ago

@edegaudenzi thanks for raising this. I'm really not sure the best way to proceed here, thoughts?

edegaudenzi commented 3 days ago

Unfortunately I don't have a precise idea either! Also, on the Guzzle project they didn't answer. However, in my own code I've managed to temporarily patch the behaviour adding an extra str_replace() before shooting the request, which is what Guzzle already does with = and & characters:

$strContactName = str_replace('+', '%2B', $strContactName);
return $objXeroAPIClient->load(\XeroPHP\Models\Accounting\Contact::class)->where('Name', $strContactName)->execute();

which - I think - in these Xero search-requests specific scenario is more than acceptable. 'calcinai/xero-php' allows us to pass exactly what you want to search rather than being worried about encoding it, meaning:

if I want to search for 'Canal+ SARL', xero-php (or its dependencies, in this case \GuzzleHttp\Psr7\Uri::withQueryValues) should make the magic behind the scenes to transform it into 'Canal%2B%20SARL', which is the format Xero likes; having said this, I have a feeling \XeroPHP\Remote\Query::execute() and \XeroPHP\Remote\Request::send() are good starting points to start thinking.