Review Amos =============== ezcHttpPassthrough::run: - It throws an exception when a file is not found, but the filename is not part of the exception. Without this it is hard to know why it failed. - HTTP_RANGE implementation is incomplete, it needs to check for multiple ranges and serve all of the specified ranges, not just one. Also it always feeds to the end of the file allthough the range might tell it to stop after a certain amount of bytes. Basically fpassthru() cannot be used since it does not accept a length parameter. - Some minor CS issues. - Output buffering is not turned off by the function, either add this code or make a HUGE note about that needs to be done before calling it. Maybe a static property to control it? - Would be nice to get some test for this, especially when fixing the HTTP_RANGE. (Too bad we can't mock the PHP functions). ezHttpUrl: - The 'query' part of parse_url result should be further parsed into an array of parameters instead of a text string. This will make it much easier to modify such queries. - 'user' and 'password' is missing. - I didn't see the point of the 'scriptName' property, it is just the basename of the path. Remove it. - The $url parameter should be optional (default to null). This makes it possible to create url strings when you already have the 'host', 'path' etc. separate. $url = new ezcHttpUrl(); $url->hostname = 'example.com'; - Needs more tests: 1. it should create the object with a given url and then examine all properties for expected values. 2. create url object with string then modify one or more properties, then examine built url 3. insert invalid data to properties, expects exceptions ezcHttpUrl::build: - The scriptParameters is concated without any encoding. I think it is better to build this from the parameter array (see above), maybe using http_build_query. - No validation or encoding of protocol, host, port, path or anchor as well. Invalid urls can be made from this. The best is to perform validation when setting the properties and throwing an exception. - 'port' should not be added if there is no 'host', if this is the case an exception should be thrown. - 'user' and 'password' is missing, password should not be used if 'user' is missing. ezcHttpUrl::prepend: - The name needs to be changed, this sounds like it will modify the 'path' element but instead it is a static function returning a string. - Parameter $uri is not a uri object but a path string, this should be a uri object. Without the object it only allows for making 'path' style urls. Proposed changes: - Change function to non-static and add the $prefixName to the constructor $url = new ezcHttpUrl( $textUrl, "image" ); $textUrl = $url->build(); - Add new method to change the $prefixName of the object. $url->setPrefixName( "site" ); - Optionally make a static helper function for quickly building new url strings. $urlText = ezcHttpUrl::buildUrlString( $textUrl, "image" );