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

Improve code quality #186

Merged
merged 29 commits into from
Oct 15, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3bb0f48
Unify redirect check in parse_response and Response
rmccue Oct 14, 2015
3bdec14
Allow passing null as $data
rmccue Oct 14, 2015
140f8f3
Improve usage of $options['type']
rmccue Oct 14, 2015
ade1b0c
Fix documentation for parse_multiple
rmccue Oct 14, 2015
321305f
Shorten line length in parse_multiple
rmccue Oct 14, 2015
b29d69e
Add corresponding exception classes for 304, 305, 306
rmccue Oct 14, 2015
1ba010e
Remove unused variable
rmccue Oct 14, 2015
e2dd4b4
Improve more code quality
rmccue Oct 14, 2015
544a1a9
Check that the URL can be parsed before use
rmccue Oct 14, 2015
b96e9db
Send $out to fsockopen.after_send, not an unused variable
rmccue Oct 14, 2015
faa0fbe
Improve quality for Requests_Cookie
rmccue Oct 14, 2015
890d510
Change $fp to $socket for clarity
rmccue Oct 14, 2015
aa3cdcb
Pass no parameters to curl.after_send
rmccue Oct 14, 2015
e614232
Improve code quality of IDNAEncoder
rmccue Oct 14, 2015
68337c0
Bring IPv6 class in line with standards
rmccue Oct 14, 2015
684de09
Bring IRI class in line with standards
rmccue Oct 14, 2015
6d7b7d7
Remove die from Requests_IRI
rmccue Oct 14, 2015
e12a2b1
Simplify IPv6::uncompress
rmccue Oct 14, 2015
216e678
Use sprintf instead of concatenation
rmccue Oct 14, 2015
4825828
Use new parse_from_headers
rmccue Oct 14, 2015
77ea59e
Correct documentation for Response
rmccue Oct 14, 2015
0fbed0f
Add missing braces
rmccue Oct 14, 2015
3626938
Clean up code per coding standards
rmccue Oct 15, 2015
063384b
Document available properties on IRI
rmccue Oct 15, 2015
6a81795
Clarify i* vs * properties in IRI
rmccue Oct 15, 2015
49ee946
Clarify we're allowed null for $data
rmccue Oct 15, 2015
e436a4a
Remove TODO
rmccue Oct 15, 2015
10a93a0
Fix inline docs in Cookie
rmccue Oct 15, 2015
04e4167
Fix more documentation and quality :sparkles:
rmccue Oct 15, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 49 additions & 39 deletions library/Requests.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,9 @@ protected static function get_transport($capabilities = array()) {

// Find us a working transport
foreach (self::$transports as $class) {
if (!class_exists($class))
if (!class_exists($class)) {
continue;
}

$result = call_user_func(array($class, 'test'), $capabilities);
if ($result) {
Expand All @@ -188,7 +189,7 @@ protected static function get_transport($capabilities = array()) {
if (self::$transport[$cap_string] === null) {
throw new Requests_Exception('No working transports found', 'notransport', self::$transports);
}

return new self::$transport[$cap_string]();
}

Expand Down Expand Up @@ -305,7 +306,7 @@ public static function patch($url, $headers, $data = array(), $options = array()
*
* @param string $url URL to request
* @param array $headers Extra headers to send with the request
* @param array $data Data to send either as a query string for GET/HEAD requests, or in the body for POST requests
* @param array|null $data Data to send either as a query string for GET/HEAD requests, or in the body for POST requests
* @param string $type HTTP request type (use Requests constants)
* @param array $options Options for the request (see description for more information)
* @return Requests_Response
Expand All @@ -326,7 +327,8 @@ public static function request($url, $headers = array(), $data = array(), $type
if (is_string($options['transport'])) {
$transport = new $transport();
}
} else {
}
else {
$need_ssl = (0 === stripos($url, 'https://'));
$capabilities = array('ssl' => $need_ssl);
$transport = self::get_transport($capabilities);
Expand Down Expand Up @@ -472,7 +474,7 @@ protected static function get_default_options($multirequest = false) {
'idn' => true,
'hooks' => null,
'transport' => null,
'verify' => dirname( __FILE__ ) . '/Requests/Transport/cacert.pem',
'verify' => dirname(__FILE__) . '/Requests/Transport/cacert.pem',
'verifyname' => true,
);
if ($multirequest !== false) {
Expand All @@ -486,7 +488,7 @@ protected static function get_default_options($multirequest = false) {
*
* @param string $url URL to request
* @param array $headers Extra headers to send with the request
* @param array $data Data to send either as a query string for GET/HEAD requests, or in the body for POST requests
* @param array|null $data Data to send either as a query string for GET/HEAD requests, or in the body for POST requests
* @param string $type HTTP request type
* @param array $options Options for the request
* @return array $options
Expand Down Expand Up @@ -601,19 +603,19 @@ protected static function parse_response($headers, $url, $req_headers, $req_data

$options['hooks']->dispatch('requests.before_redirect_check', array(&$return, $req_headers, $req_data, $options));

if ((in_array($return->status_code, array(300, 301, 302, 303, 307)) || $return->status_code > 307 && $return->status_code < 400) && $options['follow_redirects'] === true) {
if ($return->is_redirect() && $options['follow_redirects'] === true) {
if (isset($return->headers['location']) && $options['redirected'] < $options['redirects']) {
if ($return->status_code === 303) {
$options['type'] = Requests::GET;
$options['type'] = self::GET;
}
$options['redirected']++;
$location = $return->headers['location'];
if (strpos ($location, 'http://') !== 0 && strpos ($location, 'https://') !== 0) {
if (strpos($location, 'http://') !== 0 && strpos($location, 'https://') !== 0) {
// relative redirect, for compatibility make it absolute
$location = Requests_IRI::absolutize($url, $location);
$location = $location->uri;
}
$redirected = self::request($location, $req_headers, $req_data, false, $options);
$redirected = self::request($location, $req_headers, $req_data, $options['type'], $options);
$redirected->history[] = $return;
return $redirected;
}
Expand All @@ -634,13 +636,17 @@ protected static function parse_response($headers, $url, $req_headers, $req_data
* Internal use only. Converts a raw HTTP response to a Requests_Response
* while still executing a multiple request.
*
* @param string $headers Full response text including headers and body
* @param string $response Full response text including headers and body (will be overwritten with Response instance)
* @param array $request Request data as passed into {@see Requests::request_multiple()}
* @return null `$response` is either set to a Requests_Response instance, or a Requests_Exception object
*/
public static function parse_multiple(&$response, $request) {
try {
$response = self::parse_response($response, $request['url'], $request['headers'], $request['data'], $request['options']);
$url = $request['url'];
$headers = $request['headers'];
$data = $request['data'];
$options = $request['options'];
$response = self::parse_response($response, $url, $headers, $data, $options);
}
catch (Requests_Exception $e) {
$response = $e;
Expand All @@ -663,7 +669,7 @@ protected static function decode_chunked($data) {
$encoded = $data;

while (true) {
$is_chunked = (bool) preg_match( '/^([0-9a-f]+)[^\r\n]*\r\n/i', $encoded, $matches );
$is_chunked = (bool) preg_match('/^([0-9a-f]+)[^\r\n]*\r\n/i', $encoded, $matches);
if (!$is_chunked) {
// Looks like it's not chunked after all
return $data;
Expand All @@ -676,7 +682,7 @@ protected static function decode_chunked($data) {
}

$chunk_length = strlen($matches[0]);
$decoded .= $part = substr($encoded, $chunk_length, $length);
$decoded .= substr($encoded, $chunk_length, $length);
$encoded = substr($encoded, $chunk_length + $length + 2);

if (trim($encoded) === '0' || empty($encoded)) {
Expand All @@ -698,7 +704,7 @@ protected static function decode_chunked($data) {
public static function flatten($array) {
$return = array();
foreach ($array as $key => $value) {
$return[] = "$key: $value";
$return[] = sprintf('%s: %s', $key, $value);
}
return $return;
}
Expand All @@ -720,7 +726,6 @@ public static function flattern($array) {
* Implements gzip, compress and deflate. Guesses which it is by attempting
* to decode.
*
* @todo Make this smarter by defaulting to whatever the headers say first
* @param string $data Compressed data in one of the above formats
* @return string Decompressed string
*/
Expand Down Expand Up @@ -769,23 +774,26 @@ public static function decompress($data) {
public static function compatible_gzinflate($gzData) {
// Compressed data might contain a full zlib header, if so strip it for
// gzinflate()
if ( substr($gzData, 0, 3) == "\x1f\x8b\x08" ) {
if (substr($gzData, 0, 3) == "\x1f\x8b\x08") {
$i = 10;
$flg = ord( substr($gzData, 3, 1) );
if ( $flg > 0 ) {
if ( $flg & 4 ) {
list($xlen) = unpack('v', substr($gzData, $i, 2) );
$flg = ord(substr($gzData, 3, 1));
if ($flg > 0) {
if ($flg & 4) {
list($xlen) = unpack('v', substr($gzData, $i, 2));
$i = $i + 2 + $xlen;
}
if ( $flg & 8 )
if ($flg & 8) {
$i = strpos($gzData, "\0", $i) + 1;
if ( $flg & 16 )
}
if ($flg & 16) {
$i = strpos($gzData, "\0", $i) + 1;
if ( $flg & 2 )
}
if ($flg & 2) {
$i = $i + 2;
}
}
$decompressed = self::compatible_gzinflate( substr( $gzData, $i ) );
if ( false !== $decompressed ) {
$decompressed = self::compatible_gzinflate(substr($gzData, $i));
if (false !== $decompressed) {
return $decompressed;
}
}
Expand All @@ -801,55 +809,57 @@ public static function compatible_gzinflate($gzData) {
$huffman_encoded = false;

// low nibble of first byte should be 0x08
list( , $first_nibble ) = unpack( 'h', $gzData );
list(, $first_nibble) = unpack('h', $gzData);

// First 2 bytes should be divisible by 0x1F
list( , $first_two_bytes ) = unpack( 'n', $gzData );
list(, $first_two_bytes) = unpack('n', $gzData);

if ( 0x08 == $first_nibble && 0 == ( $first_two_bytes % 0x1F ) )
if (0x08 == $first_nibble && 0 == ($first_two_bytes % 0x1F)) {
$huffman_encoded = true;
}

if ( $huffman_encoded ) {
if ( false !== ( $decompressed = @gzinflate( substr( $gzData, 2 ) ) ) )
if ($huffman_encoded) {
if (false !== ($decompressed = @gzinflate(substr($gzData, 2)))) {
return $decompressed;
}
}

if ( "\x50\x4b\x03\x04" == substr( $gzData, 0, 4 ) ) {
if ("\x50\x4b\x03\x04" == substr($gzData, 0, 4)) {
// ZIP file format header
// Offset 6: 2 bytes, General-purpose field
// Offset 26: 2 bytes, filename length
// Offset 28: 2 bytes, optional field length
// Offset 30: Filename field, followed by optional field, followed
// immediately by data
list( , $general_purpose_flag ) = unpack( 'v', substr( $gzData, 6, 2 ) );
list(, $general_purpose_flag) = unpack('v', substr($gzData, 6, 2));

// If the file has been compressed on the fly, 0x08 bit is set of
// the general purpose field. We can use this to differentiate
// between a compressed document, and a ZIP file
$zip_compressed_on_the_fly = ( 0x08 == (0x08 & $general_purpose_flag ) );
$zip_compressed_on_the_fly = (0x08 == (0x08 & $general_purpose_flag));

if ( ! $zip_compressed_on_the_fly ) {
if (!$zip_compressed_on_the_fly) {
// Don't attempt to decode a compressed zip file
return $gzData;
}

// Determine the first byte of data, based on the above ZIP header
// offsets:
$first_file_start = array_sum( unpack( 'v2', substr( $gzData, 26, 4 ) ) );
if ( false !== ( $decompressed = @gzinflate( substr( $gzData, 30 + $first_file_start ) ) ) ) {
$first_file_start = array_sum(unpack('v2', substr($gzData, 26, 4)));
if (false !== ($decompressed = @gzinflate(substr($gzData, 30 + $first_file_start)))) {
return $decompressed;
}
return false;
}

// Finally fall back to straight gzinflate
if ( false !== ( $decompressed = @gzinflate( $gzData ) ) ) {
if (false !== ($decompressed = @gzinflate($gzData))) {
return $decompressed;
}

// Fallback for all above failing, not expected, but included for
// debugging and preventing regressions and to track stats
if ( false !== ( $decompressed = @gzinflate( substr( $gzData, 2 ) ) ) ) {
if (false !== ($decompressed = @gzinflate(substr($gzData, 2)))) {
return $decompressed;
}

Expand Down
2 changes: 1 addition & 1 deletion library/Requests/Auth/Basic.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function curl_before_send(&$handle) {
* @param string $out HTTP header string
*/
public function fsockopen_header(&$out) {
$out .= "Authorization: Basic " . base64_encode($this->getAuthString()) . "\r\n";
$out .= sprintf("Authorization: Basic %s\r\n", base64_encode($this->getAuthString()));
}

/**
Expand Down
33 changes: 15 additions & 18 deletions library/Requests/Cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,26 @@
*/
class Requests_Cookie {
/**
*
* Cookie name.
*
* @var string
*/
public $name;

/**
* Cookie value.
*
* @var string
*/
public $value;

/**
* Cookie attributes
*
*
* Valid keys are (currently) path, domain, expires, max-age, secure and
* httponly.
*
* @var array
* @var Requests_Utility_CaseInsensitiveDictionary|array Array-like object
*/
public $attributes = array();

Expand Down Expand Up @@ -59,7 +62,7 @@ class Requests_Cookie {
*
* @param string $name
* @param string $value
* @param array $attributes Associative array of attribute data
* @param array|Requests_Utility_CaseInsensitiveDictionary $attributes Associative array of attribute data
*/
public function __construct($name, $value, $attributes = array(), $flags = array(), $reference_time = null) {
$this->name = $name;
Expand Down Expand Up @@ -122,11 +125,7 @@ public function uri_matches(Requests_IRI $uri) {
return false;
}

if (!empty($this->attributes['secure']) && $uri->scheme !== 'https') {
return false;
}

return true;
return empty($this->attributes['secure']) || $uri->scheme === 'https';
}

/**
Expand Down Expand Up @@ -172,12 +171,8 @@ public function domain_matches($string) {
return false;
}

if (preg_match('#^(.+\.)\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$#', $string)) {
// The string should be a host name (i.e., not an IP address).
return false;
}

return true;
// The string should be a host name (i.e., not an IP address).
return !preg_match('#^(.+\.)\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$#', $string);
}

/**
Expand Down Expand Up @@ -429,18 +424,20 @@ public static function parse($string, $name = '', $reference_time = null) {
/**
* Parse all Set-Cookie headers from request headers
*
* @param Requests_Response_Headers $headers
* @param Requests_Response_Headers $headers Headers to parse from
* @param Requests_IRI|null $origin URI for comparing cookie origins
* @param int|null $time Reference time for expiration calculation
* @return array
*/
public static function parse_from_headers(Requests_Response_Headers $headers, Requests_IRI $origin = null, $reference_time = null) {
public static function parse_from_headers(Requests_Response_Headers $headers, Requests_IRI $origin = null, $time = null) {
$cookie_headers = $headers->getValues('Set-Cookie');
if (empty($cookie_headers)) {
return array();
}

$cookies = array();
foreach ($cookie_headers as $header) {
$parsed = self::parse($header, '', $reference_time);
$parsed = self::parse($header, '', $time);

// Default domain/path attributes
if (empty($parsed->attributes['domain']) && !empty($origin)) {
Expand Down
11 changes: 6 additions & 5 deletions library/Requests/Cookie/Jar.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ public function offsetExists($key) {
* @return string Item value
*/
public function offsetGet($key) {
if (!isset($this->cookies[$key]))
if (!isset($this->cookies[$key])) {
return null;
}

return $this->cookies[$key];
}
Expand Down Expand Up @@ -132,7 +133,7 @@ public function register(Requests_Hooker $hooks) {
* @param array $options
*/
public function before_request($url, &$headers, &$data, &$type, &$options) {
if ( ! $url instanceof Requests_IRI ) {
if (!$url instanceof Requests_IRI) {
$url = new Requests_IRI($url);
}

Expand All @@ -146,7 +147,7 @@ public function before_request($url, &$headers, &$data, &$type, &$options) {
continue;
}

if ( $cookie->domain_matches( $url->host ) ) {
if ($cookie->domain_matches($url->host)) {
$cookies[] = $cookie->format_for_header();
}
}
Expand All @@ -162,11 +163,11 @@ public function before_request($url, &$headers, &$data, &$type, &$options) {
*/
public function before_redirect_check(Requests_Response &$return) {
$url = $return->url;
if ( ! $url instanceof Requests_IRI ) {
if (!$url instanceof Requests_IRI) {
$url = new Requests_IRI($url);
}

$cookies = Requests_Cookie::parseFromHeaders($return->headers, $url);
$cookies = Requests_Cookie::parse_from_headers($return->headers, $url);
$this->cookies = array_merge($this->cookies, $cookies);
$return->cookies = $this;
}
Expand Down
Loading