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

Conceptual flaw in initial datasource execution #14

Closed
michael-e opened this issue Jan 8, 2015 · 2 comments
Closed

Conceptual flaw in initial datasource execution #14

michael-e opened this issue Jan 8, 2015 · 2 comments

Comments

@michael-e
Copy link
Member

While playing around with this, I noticed that some of my custom datasources didn't work correctly anymore. These datasources need some param pool values inside the execute function.

So I noticed that there is a conceptual flaw with output parameters in these lines. Basically the datasource gets executed with an empty parameter pool, which is pretty dangerous.

Since my "playground extension" has moved away from the cacheable datasource extension, I am not sending a pull request. But I am posting the solution here, so somebody can integrate it.

Current code:

// Backup the param pool, and see what's been added
$tmp = array();

// Fetch the contents
$xml = $this->__executeDatasource($ds, $tmp);

$output_params = null;

// Push into the params array
foreach ($tmp as $name => $value) {
    $param_pool[$name] = $value;
}

if (count($tmp) > 0) $output_params = sprintf("%s\n", serialize($tmp));

My code:

// Backup the param pool
$old_param_pool = $param_pool;

// Fetch the contents
$xml = $this->__executeDatasource($datasource, $param_pool);

// See what has been added to the param pool
$new_params = array_diff_key($param_pool, $old_param_pool);

// Create output params string (null or one line)
$output_params = null;
if (!empty($new_params)) {
    $output_params = sprintf("%s\n", serialize($new_params));
}
@brendo
Copy link
Member

brendo commented Jan 9, 2015

Nice pickup, I'm surprised it hasn't been picked up before to be honest!

@nitriques
Copy link
Member

Did someone patched it ? I do not use this extension a lot, but I like the fix ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants