Skip to content

Weston's review #1

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

Weston's review #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b5e38a9
Tighten location for sanitization of _jsonp GET param
westonruter Sep 30, 2015
ec4af5b
Remove unused variable
westonruter Sep 30, 2015
25a1c8e
Explicitly return null
westonruter Sep 30, 2015
97db4a9
Supply missing phpdoc params and return values
westonruter Sep 30, 2015
d6c873e
Define variable that may not be set
westonruter Sep 30, 2015
4915dbd
Remove unknown classes and param from rest_api_default_filters
westonruter Sep 30, 2015
3b2881e
Add @global and @return phpdoc tags
westonruter Sep 30, 2015
a463780
Indent and fix param name
westonruter Sep 30, 2015
b7e105f
Remove obsolete codingstandards-suppression for camelCased methods; f…
westonruter Sep 30, 2015
e748483
End comments in periods
westonruter Sep 30, 2015
2ba451e
Remove commented-out JsonSerializable::jsonSerialize()
westonruter Sep 30, 2015
b883ac0
Add more periods to ends of comments.
westonruter Sep 30, 2015
a7cdf8c
Remove returning value from void-returning method
westonruter Sep 30, 2015
5d7ebc4
Explicitly return null
westonruter Sep 30, 2015
490283f
Merge branch 'develop' of https://github.com/WP-API/api-core into fea…
westonruter Sep 30, 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
4 changes: 2 additions & 2 deletions extras.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function rest_output_rsd() {
}
?>
<api name="WP-API" blogID="1" preferred="false" apiLink="<?php echo esc_url( $api_root ); ?>" />
<?php
<?php
}

/**
Expand Down Expand Up @@ -270,7 +270,7 @@ function rest_get_date_with_gmt( $date, $force_utc = false ) {
*
* @since 4.4.0
*
* @param string $date Date string to parse and format.
* @param string $date_string Date string to parse and format.
* @return string Date formatted for ISO8601/RFC3339.
*/
function rest_mysql_to_rfc3339( $date_string ) {
Expand Down
4 changes: 1 addition & 3 deletions lib/infrastructure/class-wp-http-response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class WP_HTTP_Response implements WP_HTTP_ResponseInterface {
* @since 4.4.0
* @access public
*
* @param mixed $data Response data. Default null
* @param mixed $data Response data. Default null.
* @param int $status Optional. HTTP status code. Default 200.
* @param array $headers Optional. HTTP header map. Default empty array.
*/
Expand Down Expand Up @@ -167,9 +167,7 @@ public function set_data( $data ) {
*
* @return mixed Any JSON-serializable value.
*/
// @codingStandardsIgnoreStart
public function jsonSerialize() {
// @codingStandardsIgnoreEnd
return $this->get_data();
}
Copy link
Member

Choose a reason for hiding this comment

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

This one has to be ignored for WPCS, otherwise it throws a naming error, last I checked?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not since February: WordPress/WordPress-Coding-Standards#307

Copy link
Member

Choose a reason for hiding this comment

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

Neato, thanks!

}
13 changes: 0 additions & 13 deletions lib/infrastructure/class-wp-http-responseinterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,4 @@ public function get_status();
* @return mixed Response data.
*/
public function get_data();

/**
* @todo: Remove since it's commented out?
*
* Retrieves the response data for JSON serialization.
*
* It is expected that in most implementations, this will return the same as
* {@see get_data()}, however this may be different if you want to do custom
* JSON data handling.
*
* @return mixed Any JSON-serializable value
*/
// public function jsonSerialize();
}
32 changes: 12 additions & 20 deletions lib/infrastructure/class-wp-rest-request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public function __construct( $method = '', $route = '', $attributes = array() )
'POST' => array(),
'FILES' => array(),

// See parse_json_params
// See parse_json_params.
'JSON' => null,

'defaults' => array(),
Expand Down Expand Up @@ -326,7 +326,7 @@ public function get_content_type() {
return null;
}

// Parse type and subtype out
// Parse type and subtype out.
list( $type, $subtype ) = explode( '/', $value, 2 );

$data = compact( 'value', 'type', 'subtype', 'parameters' );
Expand All @@ -351,7 +351,7 @@ protected function get_parameter_order() {

$this->parse_json_params();

// Ensure we parse the body data
// Ensure we parse the body data.
$body = $this->get_body();
if ( $this->method !== 'POST' && ! empty( $body ) ) {
$this->parse_body_params();
Expand Down Expand Up @@ -782,7 +782,7 @@ public function sanitize_params() {

$attributes = $this->get_attributes();

// No arguments set, skip sanitizing
// No arguments set, skip sanitizing.
if ( empty( $attributes['args'] ) ) {
return true;
}
Expand All @@ -794,12 +794,13 @@ public function sanitize_params() {
continue;
}
foreach ( $this->params[ $type ] as $key => $value ) {
// check if this param has a sanitize_callback added
// Check if this param has a sanitize_callback added.
if ( isset( $attributes['args'][ $key ] ) && ! empty( $attributes['args'][ $key ]['sanitize_callback'] ) ) {
$this->params[ $type ][ $key ] = call_user_func( $attributes['args'][ $key ]['sanitize_callback'], $value, $this, $key );
}
}
}
return null;
}

/**
Expand Down Expand Up @@ -871,12 +872,10 @@ public function has_valid_params() {
* @since 4.4.0
* @access public
*
* @param string $key Parameter name.
* @param string $offset Parameter name.
* @return bool Whether the parameter is set.
*/
// @codingStandardsIgnoreStart
public function offsetExists( $offset ) {
// @codingStandardsIgnoreEnd
$order = $this->get_parameter_order();

foreach ( $order as $type ) {
Expand All @@ -894,12 +893,10 @@ public function offsetExists( $offset ) {
* @since 4.4.0
* @access public
*
* @param string $key Parameter name.
* @param string $offset Parameter name.
* @return mixed|null Value if set, null otherwise.
*/
// @codingStandardsIgnoreStart
public function offsetGet( $offset ) {
// @codingStandardsIgnoreEnd
return $this->get_param( $offset );
}

Expand All @@ -909,13 +906,11 @@ public function offsetGet( $offset ) {
* @since 4.4.0
* @access public
*
* @param string $key Parameter name.
* @param mixed $value Parameter value.
* @param string $offset Parameter name.
* @param mixed $value Parameter value.
*/
// @codingStandardsIgnoreStart
public function offsetSet( $offset, $value ) {
// @codingStandardsIgnoreEnd
return $this->set_param( $offset, $value );
$this->set_param( $offset, $value );
}

/**
Expand All @@ -924,12 +919,9 @@ public function offsetSet( $offset, $value ) {
* @since 4.4.0
* @access public
*
* @param string $key Parameter name.
* @param mixed $value Parameter value.
* @param string $offset Parameter name.
*/
// @codingStandardsIgnoreStart
public function offsetUnset( $offset ) {
// @codingStandardsIgnoreEnd
$order = $this->get_parameter_order();

// Remove the offset from every group.
Expand Down
10 changes: 5 additions & 5 deletions lib/infrastructure/class-wp-rest-response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function add_link( $rel, $href, $attributes = array() ) {
}

if ( isset( $attributes['href'] ) ) {
// Remove the href attribute, as it's used for the main URL
// Remove the href attribute, as it's used for the main URL.
unset( $attributes['href'] );
}

Expand Down Expand Up @@ -116,7 +116,7 @@ public function remove_link( $rel, $href = null ) {
*/
public function add_links( $links ) {
foreach ( $links as $rel => $set ) {
// If it's a single link, wrap with an array for consistent handling
// If it's a single link, wrap with an array for consistent handling.
if ( isset( $set['href'] ) ) {
$set = array( $set );
}
Expand Down Expand Up @@ -150,8 +150,8 @@ public function get_links() {
* @link http://tools.ietf.org/html/rfc5988
* @link http://www.iana.org/assignments/link-relations/link-relations.xml
*
* @param string $rel Link relation. Either an IANA registered type, or an absolute URL
* @param string $link Target IRI for the link
* @param string $rel Link relation. Either an IANA registered type, or an absolute URL.
* @param string $link Target IRI for the link.
* @param array $other Optional. Other parameters to send, as an assocative array.
* Default empty array.
*/
Expand All @@ -164,7 +164,7 @@ public function link_header( $rel, $link, $other = array() ) {
}
$header .= '; ' . $key . '=' . $value;
}
return $this->header( 'Link', $header, false );
$this->header( 'Link', $header, false );
}

/**
Expand Down
63 changes: 45 additions & 18 deletions lib/infrastructure/class-wp-rest-server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public function check_authentication() {
* @access protected
*
* @param WP_Error $error WP_Error instance.
* @return array List of associative arrays with code and message keys.
* @return WP_REST_Response List of associative arrays with code and message keys.
*/
protected function error_to_response( $error ) {
$error_data = $error->get_error_data();
Expand Down Expand Up @@ -270,8 +270,8 @@ protected function error_to_response( $error ) {
* @since 4.4.0
* @access protected
*
* @param string $code WP_Error-style code
* @param string $message Human-readable message
* @param string $code WP_Error-style code.
* @param string $message Human-readable message.
* @param int $status Optional. HTTP status code to send. Default null.
* @return string JSON representation of the error
*/
Expand Down Expand Up @@ -331,6 +331,8 @@ public function serve_request( $path = null ) {
*/
$jsonp_enabled = apply_filters( 'rest_jsonp_enabled', true );

$jsonp_callback = null;

if ( ! $enabled ) {
echo $this->json_error( 'rest_disabled', __( 'The REST API is disabled on this site.' ), 404 );
return false;
Expand All @@ -341,8 +343,14 @@ public function serve_request( $path = null ) {
return false;
}

// Check for invalid characters (only alphanumeric allowed)
if ( ! is_string( $_GET['_jsonp'] ) || preg_match( '/[^\w\.]/', $_GET['_jsonp'] ) ) {
// Check for invalid characters (only alphanumeric allowed).
if ( is_string( $_GET['_jsonp'] ) ) {
$jsonp_callback = preg_replace( '/[^\w\.]/', '', wp_unslash( $_GET['_jsonp'] ), -1, $illegal_char_count );
if ( 0 !== $illegal_char_count ) {
$jsonp_callback = null;
}
}
if ( null === $jsonp_callback ) {
echo $this->json_error( 'rest_callback_invalid', __( 'The JSONP callback function is invalid.' ), 400 );
return false;
}
Expand Down Expand Up @@ -432,7 +440,7 @@ public function serve_request( $path = null ) {

if ( ! $served ) {
if ( 'HEAD' === $request->get_method() ) {
return;
return null;
}

// Embed links inside the request.
Expand All @@ -447,14 +455,15 @@ public function serve_request( $path = null ) {
$result = wp_json_encode( $result->data[0] );
}

if ( isset( $_GET['_jsonp'] ) ) {
if ( $jsonp_callback ) {
// Prepend '/**/' to mitigate possible JSONP Flash attacks
// http://miki.it/blog/2014/7/8/abusing-jsonp-with-rosetta-flash/
echo '/**/' . $_GET['_jsonp'] . '(' . $result . ')';
echo '/**/' . $jsonp_callback . '(' . $result . ')';
} else {
echo $result;
}
}
return null;
}

/**
Expand All @@ -463,9 +472,14 @@ public function serve_request( $path = null ) {
* @since 4.4.0
* @access public
*
* @param WP_REST_Response $response Response object
* @param WP_REST_Response $response Response object.
* @param bool $embed Whether links should be embedded.
* @return array
* @return array {
* Data with sub-requests embedded.
*
* @type array [$_links] Links.
* @type array [$_embedded] Embeddeds.
* }
*/
public function response_to_data( $response, $embed ) {
$data = $this->prepare_response( $response->get_data() );
Expand Down Expand Up @@ -529,7 +543,12 @@ public static function get_response_links( $response ) {
* @access protected
*
* @param array $data Data from the request.
* @return array Data with sub-requests embedded.
* @return array {
* Data with sub-requests embedded.
*
* @type array [$_links] Links.
* @type array [$_embedded] Embeddeds.
* }
*/
protected function embed_links( $data ) {
if ( empty( $data['_links'] ) ) {
Expand Down Expand Up @@ -614,7 +633,7 @@ protected function embed_links( $data ) {
* @since 4.4.0
* @access public
*
* @param WP_REST_Response $response Response object
* @param WP_REST_Response $response Response object.
* @param bool $embed Whether links should be embedded.
* @return WP_REST_Response New response with wrapped data
*/
Expand Down Expand Up @@ -645,6 +664,7 @@ public function envelope_response( $response, $embed ) {
* @since 4.4.0
* @access public
*
* @param string $namespace Namespace.
* @param string $route The REST route.
* @param array $route_args Route arguments.
* @param bool $override Optional. Whether the route should be overriden if it already exists.
Expand Down Expand Up @@ -752,6 +772,8 @@ public function get_routes() {
$methods = explode( ',', $handler['methods'] );
} else if ( is_array( $handler['methods'] ) ) {
$methods = $handler['methods'];
} else {
$methods = array();
}

$handler['methods'] = array();
Expand Down Expand Up @@ -828,7 +850,7 @@ public function dispatch( $request ) {

foreach ( $this->get_routes() as $route => $handlers ) {
foreach ( $handlers as $handler ) {
$callback = $handler['callback'];
$callback = $handler['callback'];
$response = null;

if ( empty( $handler['methods'][ $method ] ) ) {
Expand Down Expand Up @@ -929,7 +951,7 @@ public function dispatch( $request ) {
*
* @return bool|string Boolean false or string error message.
*/
protected function get_json_last_error( ) {
protected function get_json_last_error() {
// See https://core.trac.wordpress.org/ticket/27799.
if ( ! function_exists( 'json_last_error' ) ) {
return false;
Expand All @@ -954,6 +976,11 @@ protected function get_json_last_error( ) {
* @since 4.4.0
* @access public
*
* @param array $request {
* Request.
*
* @type string $context Context.
* }
* @return array Index entity
*/
public function get_index( $request ) {
Expand Down Expand Up @@ -1011,7 +1038,7 @@ public function get_namespace_index( $request ) {
);
$response = rest_ensure_response( $data );

// Link to the root index
// Link to the root index.
$response->add_link( 'up', rest_url( '/' ) );

/**
Expand All @@ -1034,7 +1061,7 @@ public function get_namespace_index( $request ) {
* @since 4.4.0
* @access public
*
* @param array $routes Routes to get data for
* @param array $routes Routes to get data for.
* @param string $context Optional. Context for data. Accepts 'view' or 'help'. Default 'view'.
* @return array Route data to expose in indexes.
*/
Expand Down Expand Up @@ -1165,8 +1192,8 @@ protected function set_status( $code ) {
* @since 4.4.0
* @access public
*
* @param string $key Header key
* @param string $value Header value
* @param string $key Header key.
* @param string $value Header value.
*/
public function send_header( $key, $value ) {
/*
Expand Down
Loading