Skip to content

Commit

Permalink
Fix warnings from file_get_contents() in Xml::build()
Browse files Browse the repository at this point in the history
Use HttpSocket to get proper exceptions when trying to load XML from
remote servers.

Fixes #3379
  • Loading branch information
markstory committed Nov 19, 2012
1 parent 6b4afb9 commit fb275c5
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/Cake/Test/Case/Utility/XmlTest.php
Expand Up @@ -177,6 +177,7 @@ public static function invalidDataProvider() {
array(null),
array(false),
array(''),
array('http://localhost/notthere.xml'),
);
}

Expand Down
13 changes: 10 additions & 3 deletions lib/Cake/Utility/Xml.php
Expand Up @@ -18,6 +18,7 @@
* @since CakePHP v .0.10.3.1400
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
App::uses('HttpSocket', 'Network/Http');

/**
* XML handling for Cake.
Expand Down Expand Up @@ -97,9 +98,15 @@ public static function build($input, $options = array()) {
return self::fromArray((array)$input, $options);
} elseif (strpos($input, '<') !== false) {
return self::_loadXml($input, $options);
} elseif (file_exists($input) || strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
$input = file_get_contents($input);
return self::_loadXml($input, $options);
} elseif (file_exists($input)) {
return self::_loadXml(file_get_contents($input), $options);
} elseif (strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
$socket = new HttpSocket();
$response = $socket->get($input);
if (!$response->isOk()) {
throw new XmlException(__d('cake_dev', 'XML cannot be read.'));
}
return self::_loadXml($response->body, $options);
} elseif (!is_string($input)) {
throw new XmlException(__d('cake_dev', 'Invalid input.'));
}
Expand Down

4 comments on commit fb275c5

@renan
Copy link
Contributor

@renan renan commented on fb275c5 Jan 17, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a small BC as file_get_contents follow redirects by default and HttpSocket doesn't.

@dereuromark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_get_contents($input) should probably use the $context (second param) and connection: close, though, as HTTP1.1 now leaves it open with this method if not specifically declared as "closing" in the headers creating an overhead of never-closing connections in some cases.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_get_contents() was removed because of shitty error handling, I don't know why we'd add it back. HttpSocket can follow redirects its just a config option.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 689745d

Please sign in to comment.