Permalink
Browse files

Fix warnings from file_get_contents() in Xml::build()

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 fb275c5fa257c98f85883115ae12dbf5671094e1
Showing with 11 additions and 3 deletions.
  1. +1 −0 lib/Cake/Test/Case/Utility/XmlTest.php
  2. +10 −3 lib/Cake/Utility/Xml.php
@@ -177,6 +177,7 @@ public static function invalidDataProvider() {
array(null),
array(false),
array(''),
+ array('http://localhost/notthere.xml'),
);
}
View
@@ -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.
@@ -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.'));
}

4 comments on commit fb275c5

@renan

This comment has been minimized.

Show comment
Hide comment
@renan

renan Jan 17, 2013

Member

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

Member

renan replied Jan 17, 2013

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

@dereuromark

This comment has been minimized.

Show comment
Hide comment
@dereuromark

dereuromark Jan 17, 2013

Member

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.

Member

dereuromark replied Jan 17, 2013

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

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Jan 17, 2013

Member

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.

Member

markstory replied Jan 17, 2013

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

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Jan 17, 2013

Member

Fixed in 689745d

Member

markstory replied Jan 17, 2013

Fixed in 689745d

Please sign in to comment.