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

SimplePie_IRI::set_query() improperly decodes URLs passed in query string values #112

Closed
michaellehmkuhl opened this issue Jan 19, 2011 · 2 comments

Comments

@michaellehmkuhl
Copy link

commented Jan 19, 2011

Ran into an issue in 1.2.1-dev where a URL that contains a query string value that contains another URL was improperly decoded. Example of the problem:

http://test.com/?site=yahoo&url=http%3A%2F%2Fwww.yahoo.com%2F%3Fp%3Dsimplepie
was incorrectly output as
http://test.com/?site=yahoo&url=http://www.yahoo.com/?p=simplepie

The patch below provides a fix by running SimplePie_IRI::replace_invalid_with_pct_encoding() on each argument in the query string rather than the whole thing.

12170c12170,12175
<           $this->query = $this->replace_invalid_with_pct_encoding($query, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~!$\'()*+,;:@/?&=');

---
>           $args = array();
>           parse_str($query, $args);
>           foreach ($args as $k=>$v) {
>               $args[$k] = $this->replace_invalid_with_pct_encoding($v, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~!$\'()*+,;:@/?&=');
>           }
>           $this->query = http_build_query($args);
@rmccue

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2011

Does this still occur in the latest version from the one-dot-two branch?

@k1mk1m

This comment has been minimized.

Copy link

commented May 22, 2011

It does still occur. I tested latest from one-dot-two branch and also latest from master branch. Possible to reproduce with the following code:

<?php
// SimplePie 1.2 (cce56bb)
require 'simplepie-simplepie-cce56bb/simplepie.inc';
// SimplePie 1.3 (0475341)
// require 'simplepie-simplepie-0475341/SimplePieAutoloader.php';

// URL with URL-encoded value in the query string
$url = 'http://example.org/test.php?url=example.org%2Fget.php%3Fid%3D1%26type%3Darticle';
$sp = new SimplePie();
$sp->set_feed_url($url);
$sp_url = $sp->feed_url;
header('Content-Type: text/plain');
echo "Original: $url\n";
echo "SimplePie: $sp_url\n";
?>

Produces

Original: http://example.org/test.php?url=example.org%2Fget.php%3Fid%3D1%26type%3Darticle
SimplePie: http://example.org/test.php?url=example.org/get.php?id=1&type=article

The suggested patch will work for this example, but will break (never ending loop) if URLs have brackets in, e.g. if you change $url to

$url = 'http://example.org/test.php?url[]=example&url[]=test';

I'm not so familiar with IRI code, so nothing to suggest yet.

@rmccue rmccue closed this in 76b5fd6 Sep 18, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.