Soap call problem with >32bit ids

[jclee100]

Seems like the issue is not isolated to just BatchJobs. SoapCall is also affected.

The Keyword id 289674651551 became 2147483647.

Error message: [EntityNotFound.INVALID_ID @ operations[0].operand.criterion.id; trigger:'CriterionId{id=2147483647}']

Output from soap call from AdsSoapClient Line 115

<?xml version="1.0" encoding="UTF-8"?>
<SOAP-ENV:Envelope
    xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"
    xmlns:ns1="https://adwords.google.com/api/adwords/cm/v201702"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <SOAP-ENV:Header>
        <ns1:RequestHeader>
            <ns1:clientCustomerId></ns1:clientCustomerId>
            <ns1:developerToken></ns1:developerToken>
            <ns1:userAgent>Soya (AwApi-PHP, googleads-php-lib/25.3.0, PHP/7.0.12)</ns1:userAgent>
            <ns1:validateOnly>false</ns1:validateOnly>
            <ns1:partialFailure>false</ns1:partialFailure>
        </ns1:RequestHeader>
    </SOAP-ENV:Header>
    <SOAP-ENV:Body>
        <ns1:mutate>
            <ns1:operations>
                <ns1:operator>REMOVE</ns1:operator>
                <ns1:operand xsi:type="ns1:BiddableAdGroupCriterion">
                    <ns1:adGroupId>42077683873</ns1:adGroupId>
                    <ns1:criterion xsi:type="ns1:Keyword">
                        <ns1:id>2147483647</ns1:id>
                        <ns1:text>Malt Patron Pinot Grigio</ns1:text>
                        <ns1:matchType>EXACT</ns1:matchType>
                    </ns1:criterion>
                    <ns1:userStatus>REMOVED</ns1:userStatus>
                </ns1:operand>
            </ns1:operations>
        </ns1:mutate>
    </SOAP-ENV:Body>
</SOAP-ENV:Envelope>

Guess you'll have to cast id to float in the argument.

[dklimkin]

Also, some criteria across examples etc. Knack, we may need to grep across lib for "intval(" and make sure the value range may not exceed int32.

PHP documentation around Integer makes me depressed.

[fiboknacky]

Hello JC,

Thanks for reporting this!

I'm afraid that doing anything (like casting values) in AdsSoapClient is not possible, as it's too deep and we don't know the structure of an object passed there.
It can be an array of CampaignOperation, AdGroupOperation, or anything, as all services extend this class.

In fact, the $argument of the __soapCall is created and passed by users so you could control the value that will be passed to this method. Unfortunately, PHP isn't strongly typed language so even if we provide casting in a method, but your passed numbers exceeds max integer in your system, it'll not work properly. So this should be resolved by casting by users.

As an example, CampaignService's mutate() accepts $operations which is expected to be an array of CampaignOperation, which in turn incorporates Campaign.

You can cast the value of id properly to float when creating a Campaign object before passing to CampaignService's mutate(). Again, even if there is casting in the setId() of Campaign, it won't work if you still pass your number as is.

I'll document this properly so other users won't face this problem too.

Best,
Knack

[jclee100]

Thanks for the suggestions.

I am developing my app using Windows and Mac and deploying to Linux. Only Window's PHP is limited to 32 bits. So casting it myself is also just for a single machine.

I have no problems with casting it, but why can't the library cast it at the service classes' mutate method by passing the operations through a function to check for the following?

1. integers > 32bit
2. and checking PHP_INT_SIZE

Again, even if there is casting in the setId() of Campaign, it won't work if you still pass your number as is.

Can you explain why?

[fiboknacky]

Hello JC,

Provided that I have a setId() as the following:

public function setId($id) {
  $this->id = (PHP_INT_SIZE === 4) ? floatval($id) : $id;
  return $this;
}

And given that you pass a variable exceeding 2147483647 like 2147485000, to this function:

// Case 1
$a = 2147485000; // would work
$campaign->setId($a);
// Case 2
$a = "2147485000"; // would work
$campaign->setId($a);
// Case 3
$a = intval(2147485000); // not work
$campaign->setId($a);

My point is that it still depends on the values that you pass to the method.
If you pass like case 1, it should work without any casting. PHP should determine that the value is larger than integer, and it will cast that to float automatically. Case 2 should work fine as well as it's a string.

For case 3, it wouldn't work because 2147485000 will be maxed out as 2147483647 before passed to the method, and casting it to float just produces floatval of 2147483647, not of 2147485000.

Best,
Knack

[fiboknacky]

And the reason why I agreed that we should fix #236 is because we explicitly did intval() on your passed values and that definitely prohibited any 32 bit integer to be casted properly.
Unlike that case, this one should be fine if you don't do intval() explicitly on your values. :)

[jclee100]

Unlike that case, this one should be fine if you don't do intval() explicitly on your values.

Maybe there's a misunderstanding, I didn't intval(). The argument to soap call reflects the correct integer. Only after soapCall the XML became 32bits.

[fiboknacky]

Hi,

So, do you mean the XMLs produced by SOAP toolkit (before sending to the servers) are not correct?
And did you face this problem in the deprecated branch?

Knack

[jclee100]

Yes. I printed out the argument and the XML to do a comparison. The XML is wrong. See similar issue here: http://stackoverflow.com/questions/19228213/php-soapclient-soap-request-with-long-integer

And

http://stackoverflow.com/questions/27890906/soapclient-converts-string-8046618015-to-2147483647-maximum-int-size-when

[fiboknacky]

Hi JC,

Thanks for information. Then it's a PHP SOAP toolkit bug itself.
I wonder in this case, casting it to float explicitly will help anything?

For example, if we have:

$a = 21474850000;
$b = flotval(21474850000);

Using vardump(), they should produce the same thing: float(2.147485e+10) in 32 bit, no?

Knack

[jclee100]

I've been researching a bit yesterday. Not sure if it's the case, but seems like no one is going to fix it: https://bugs.php.net/bug.php?id=48717

For your example, here's how it actually executed on my system.

$a = 21474850000;
$b = floatval(21474850000);
var_dump($a, $b);

// Output
float(21474850000)
float(21474850000)

[fiboknacky]

Yup, that's what I think.
So, I still don't get if casting at any point will help this as it looks like the bug in the SOAP toolkit itself.
As a final workaround I can think of, what about casting it to string instead?

After converting to XML, there are no differences between int and string and this should work fine on the API servers too.

[jclee100]

Ok, just did further investigation.

In my database I am storing the id as bigint(20). Querying using PHP gets it default as a string of the correct value i.e. "289674651551". (Apparently, default MySQL behavior.) The SoapClient then automatically converts this string into integer 2147483647.

Sorry for the confusion.

So string does not work. I can't cast it to integer either, as you said. So the only solution for me is to cast it to float and this works.

The issue still lies with SOAP toolkit.

And the question is still who should do the casting?

// From AdsSoapClient argument
          #criterion: Keyword {#483
            #text: "Malt Patron Pinot Grigio"
            #matchType: "BROAD"
            #id: "288585078631"
            #type: null
            #CriterionType: null
          }

[fiboknacky]

Hello JC,

As mentioned in previously, I'm still concerned that providing casting in the setter doesn't help all cases.
It would help in the case of string, but not for the case of integer.

Let me consider pros and cons more carefully.
And thanks for helping investigate this too!
In your case, probably it's resolved now because you have to cast the values retrieved from MySQL anyway. You could check if PHP_INT_SIZE === 4 and converts it to float instead of int.

Best,
Knack

[jclee100]

In your case, probably it's resolved now because you have to cast the values retrieved from MySQL anyway. You could check if PHP_INT_SIZE === 4 and converts it to float instead of int.

To be honest, I rather not cast this on my end as the only case where it's necessary is this issue. To change this, I'll need to either target the columns manually or change things on the framework level, both which adds to more maintenance down the line.

I'll wait for your decision on this.

In the mean time, I'll ignore it on this development machine :D

Thanks for your help!

[jclee100]

Hi Knack,

Just encountered this issue again. This time it's not related to database.

In my app I make a DELETE request like this: http://app.dev/feeds/45250724/feed_items/11688017155

The URL parameters are automatically cast to string by the routing system. So unless I manually cast it to float, the issue will occur.

It'll be inconvenient to cast it to float manually every time. So I hope that you'll add this into the library.

Thanks!

[fiboknacky]

Hi JC,

Could you please add more details like SOAP logs? I can't access your link. :)
When you say the routing system, it looks like not caused by this library though?

Best,
Knack

[jclee100]

Hi Knack,

The URL is for an AJAX request within my app.

The routing system is part of the framework I am using. So a URL like http://app.dev/feeds/45250724/feed_items/11688017155, I can access the parameters like 45250724 and 11688017155 in my controller as arguments. These arguments are received as strings.

As discussed few days ago, when strings are over 32bits the SOAP toolkit will output it as 2147483647 hence causing the issue.

It's not part of the library. But my point earlier: Unless I go to every controller method or route and cast it to float manually, the issue will occur. It's inconvenient to do so. If it's cast on the library itself, then I don't have to handle this.

The ideal solution, I think, is to run the $argument in Line 115 of AdsSoapClient.php through a function that automatically casts these numeric strings to float (if system is 32bits). This will resolve the issue globally. Or since you are generating the classes from WSDL, you would know the data type of the properties and can cast them in the classes themselves.

Hope that is clearer.

Let me know if you need more information.

Thanks!

JC

[fiboknacky]

Hello JC,

The routing system is part of the framework I am using. So a URL like http://app.dev/feeds/45250724/feed_items/11688017155, I can access the parameters like 45250724 and 11688017155 in my controller as arguments. These arguments are received as strings.
It's not part of the library. But my point earlier: Unless I go to every controller method or route and cast it to float manually, the issue will occur. It's inconvenient to do so. If it's cast on the library itself, then I don't have to handle this.

I understand your case for passing values to AdWords API server, but in this case, it looks like you mean your system also has an issue with the results returned from the API?
If so, how would providing casting values, e.g., in AdsSoapClient as you mentioned, help in this case?

Moreover,

The ideal solution, I think, is to run the $argument in Line 115 of AdsSoapClient.php through a function that automatically casts these numeric strings to float (if system is 32bits).

I don't think this would be easy to implement. It looks like we have to deeply traverse to all members of all objects stored as $argument, which would ruin performance unnecessarily.

Or since you are generating the classes from WSDL, you would know the data type of the properties and can cast them in the classes themselves.

Unfortunately, this doesn't solve all the problems. People can still run into problems if they pass 32-bit integer into setters of AdWords integer values (like campaign's ID). That's why I'm still reluctant to do this. My thought is like "garbage in, garbage out". You have a choice to pass the right values to the system and the library shouldn't change your value unnecessarily. :)
Again, this is quite different from the case of BatchJobs. In that case, even if you provide correct values, we still make changes to your values, so I think it should be fixed.

Note that, I don't turn this proposal down completely. I know that it at least helps your case.
But I wouldn't have time to commit to this very soon.

Best,
Knack

[jclee100]

Hi Knack,

I understand your case for passing values to AdWords API server, but in this case, it looks like you mean your system also has an issue with the results returned from the API?

No, my example deals with deleting a feed item by doing a DELETE request through AJAX. I do already have the ids. As the routing system interprets the URL, it reads the params like so http://app.dev/feeds/{feed_id}/feed_items/{feed_item_id} . Unless I manually cast the parameters, it'll be string. I believe this is true in most other frameworks too.

I don't think this would be easy to implement. It looks like we have to deeply traverse to all members of all objects stored as $argument, which would ruin performance unnecessarily.

While that's true, we can skip traversing if system is not 32 bits. Also, we can add a config option that switches off this feature if the developer chooses to cast it. It's not elegant but it works.

Unfortunately, this doesn't solve all the problems. People can still run into problems if they pass 32-bit integer into setters of AdWords integer values (like campaign's ID).

This is only true if the developer manually casts to integer with intval() before setting the property. In no other cases will this happen! By default the integers returned from AdWords API will be the correct even if they are greater than 32bits.

My thought is like "garbage in, garbage out". You have a choice to pass the right values to the system and the library shouldn't change your value unnecessarily. :)

Yes. However to pass the right value in this case, I'll have to cast float at every instance. While I can do that, it's inconvenient and easy to miss. I imagine the developer would expect the string value id to work. The only way to discover this problem is to do __getLastRequest() on SOAP.

It's not urgent as this is only a problem on Windows. I just need to make sure you get my points clearly to make the decision.

And of course if SOAP toolkit can fix that issue, then all these goes away :D

Thanks!

[fiboknacky]

Hi JC,

Thanks for more thoughts.

And of course if SOAP toolkit can fix that issue, then all these goes away :D

Yes. The ideal way is to have SOAP toolkit fix this.

This is only true if the developer manually casts to integer with intval() before setting the property. In no other cases will this happen! By default the integers returned from AdWords API will be the correct even if they are greater than 32bits.

As for this, if I do casting values, I'd go for this way, do the casting in the setters, rather than in AdsSoapClient. I'm sorry but it still doesn't look good to me to do anything expensively at that class.
It's also hard to test and gets its right.

No, my example deals with deleting a feed item by doing a DELETE request through AJAX. I do already have the ids. As the routing system interprets the URL, it reads the params like so http://app.dev/feeds/{feed_id}/feed_items/{feed_item_id} . Unless I manually cast the parameters, it'll be string. I believe this is true in most other frameworks too.

Sorry for this too but still doesn't follow you here. :(
I totally understand that you use feed_id and feed_item_id for your job.
But how does it relate to this particular issue we're discussing above?

In other words, it seems you don't pass feed_id into AdsSoapClient and then sent via SOAP toolkit to API servers in this example?
Instead, you use the values returned by the API in your routing system? If so, how would fixing of either the algorithm in AdsSoapClient solve your issue here?

Cheers,
Knack

[jclee100]

Hi,

Sorry for this too but still doesn't follow you here. :(
I totally understand that you use feed_id and feed_item_id for your job.
But how does it relate to this particular issue we're discussing above?

It's related because throughout the application the ids if placed into the URL are cast as strings (when received in PHP) -- and strings must be cast to float to avoid the issue. Compared to our discussion earlier where I thought it's only when queried by the database that I get strings, this is more widespread. Therefore now I must cast to float on a wider scale.

In other words, it seems you don't pass feed_id into AdsSoapClient and then sent via SOAP toolkit to API servers in this example?

I do. Using the example in my reply, the request is received in PHP as strings. Then the operations constructed and sent to the API. In other words, 11688017155 from the URL is used for an API operation which becomes 2147483647.

[fiboknacky]

Hi JC,

So,

As the routing system interprets the URL, it reads the params like so http://app.dev/feeds/{feed_id}/feed_items/{feed_item_id}

Are you saying that you get the URL in forms of the above, which is filled with {feed_id} and {feed_item_id}, then you extract them as string as pass them as is to the setter of, e.g., FeedItem?
If so, I understand what you say now.

Just wonder if you're also affected by this issue in other libraries too?

Cheers,
Knack

[jclee100]

Hi Knack,

That's what I mean. So with that example, I must go through every route and cast them to float.

I haven't yet work with an API that requires 64 bits integers other than AdWords. Or at least not that I know of - because I've been working on Mac (64bit PHP) with no issues.

JC

[fiboknacky]

Classes are generated with default floatval() for 32-bit in v28.0.0.