Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added missing is_array check #1249

Merged
merged 3 commits into from
Jul 12, 2017
Merged

Added missing is_array check #1249

merged 3 commits into from
Jul 12, 2017

Conversation

sypherlev
Copy link
Contributor

Okay, in Resources.php, it's possible for $paramSpec['repeated'] to not be set while $paramSpec['value'] is an array. This causes execution to fall through to line 280 and then throw a warning on rawurldecode($paramSpec['value']), which expects a string, and then the request to the API will fail.

Checking if $paramSpec['value'] is an array and then setting it to the first element if so solves the problem.

Not sure if this is the best solution, it's just the one I'm using right now to get the thing working at all.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 23, 2017
As per error coming from Travis CI
@bshaffer
Copy link
Contributor

I am not entirely certain what is happening here. Can you provide a repeatable test case?

@sypherlev
Copy link
Contributor Author

Sorry, I'll see if I can explain a bit more. I'm using the API client to make queries to YouTube Analytics in a system built on Yii2, as follows:

In the Yii controller constructor:

$this->client = new \Google_Client();
$this->client->setScopes([\Google_Service_YouTubeAnalytics::YT_ANALYTICS_READONLY, \Google_Service_YouTubeAnalytics::YT_ANALYTICS_MONETARY_READONLY]);
$this->client->setAuthConfig(Yii::getAlias('@root').'/client_secret.json');
$this->client->setDeveloperKey(Yii::$app->params['ytoauth_apikey']);
$this->client->setAccessType('offline');
$this->client->setApprovalPrompt('force');
$this->client->setRedirectUri('http://'.$_SERVER['HTTP_HOST'].'/api/web/youtubeapi/reportoauth');
$key = file_get_contents(Yii::getAlias('@root').'/key.json');
if(strlen($key) > 0) {
    $keyjson = json_decode($key, true);
    $this->client->setAccessToken($keyjson);
}

Then the actual query:

/* $startDate and $endDate already set in the format Y-m-d */
$youtube = new \Google_Service_YouTubeAnalytics($this->client);
$additional = ['filters' => 'video=='.implode(',', $filters)];
$additional['dimensions'] = 'month,video';
$metrics = array('estimatedMinutesWatched,averageViewDuration,views,estimatedRevenue');
$ids = 'contentOwner=='.$contentowner;
$results = $youtube->reports->query($ids, $startDate, $endDate, $metrics, $additional);

I'm condensing it a bit but that's more or less how it goes. I found that the Oauth handshake worked fine, getting and using refresh tokens worked fine, but no other queries to the API worked.

Line 275 in src/Google/Service/Resource.php has the following check:
if (isset($paramSpec['repeated']) && is_array($paramSpec['value']))

The problem is that if $paramSpec['repeated'] is not set, then execution jumps to line 280 without ever looking at whether $paramSpec['value'] is an array.

At line 280:
$queryVars[] = $paramName . '=' . rawurlencode(rawurldecode($paramSpec['value']));

This assumes $paramSpec['value'] is going to be a string. If it's not, then rawurldecode() fails with a warning and the request doesn't work. That's what happened every time I tried to run a query, hence adding in another is_array check to make sure that $paramSpec['value'] will be a string when it hits that line.

Again I'm not completely sure if setting $paramSpec['value'] to the first element in the array is a good solution at that point, but it seemed to work okay.

@bshaffer
Copy link
Contributor

bshaffer commented Jul 7, 2017

A better solution would be to check if $paramSpec['value'] is an array before adding it to $queryVars, and if so, iterating over all of them and running rawurlencode. Or better yet, implement something like this:

if (is_array($paramSpec['values'])) {
    $queryVars[] = array_map(
        'rawurlencode',
        array_map(
            'rawurldecode',
            $paramSpec['values']
        )
    );
}

@bshaffer
Copy link
Contributor

bshaffer commented Jul 7, 2017

Nevermind, I think the best thing to do here is just remove the repeated check from the conditional.

Removed the is_array check and the isset($paramSpec['repeated']) check.
@sypherlev
Copy link
Contributor Author

However it gets fixed doesn't matter to me :P I've made that change in the patch.

@bshaffer bshaffer merged commit 9bef249 into googleapis:master Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants