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.

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

Jump to bottom

Fix fetching relative stylesheet URLs #4284

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Fix fetching relative stylesheet URLs #4284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98794c4
WIP: Fix fetching relative stylesheet URLs
westonruter Feb 11, 2020
d4d44b3
Add granularity to validate cap checks
westonruter Feb 24, 2020
3f06c8e
Merge branch 'develop' of github.com:ampproject/amp-wp into fix/relat…
westonruter Jun 3, 2020
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
13 changes: 6 additions & 7 deletions includes/sanitizers/class-amp-form-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ public function sanitize() {
/**
* Get the action URL for the form element.
*
* @todo De-duplicate with AMP_Style_Sanitizer::normalize_stylesheet_url().
*
* @param string $action_url Action URL.
* @return string Action URL.
*/
Expand Down Expand Up @@ -143,28 +145,25 @@ protected function get_action_url( $action_url ) {
return $action_url;
}

// Make URL protocol relative.
$parsed_url['scheme'] = '//';

// Set an empty path if none is defined but there is a host.
if ( ! isset( $parsed_url['path'] ) && isset( $parsed_url['host'] ) ) {
$parsed_url['path'] = '';
}

if ( ! isset( $parsed_url['host'] ) ) {
$parsed_url['host'] = $_SERVER['HTTP_HOST'];
$parsed_url['host'] = $_SERVER['HTTP_HOST']; // @todo Use home_url() instead?
}

if ( ! isset( $parsed_url['path'] ) ) {
// If there is action URL path, use the one from the request.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) );
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ); // @todo This is wrong because it includes the path.
} elseif ( '' !== $parsed_url['path'] && '/' !== $parsed_url['path'][0] ) {
// If the path is relative, append it to the current request path.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ) . trailingslashit( $parsed_url['path'] );
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ) . trailingslashit( $parsed_url['path'] ); // @todo This is wrong because it includes the path.
}

// Rebuild the URL.
$action_url = $parsed_url['scheme'];
$action_url = '//';
if ( isset( $parsed_url['user'] ) ) {
$action_url .= $parsed_url['user'];
if ( isset( $parsed_url['pass'] ) ) {
Expand Down
75 changes: 72 additions & 3 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,75 @@ private function get_stylesheet_from_url( $stylesheet_url ) {
return $this->fetch_external_stylesheet( $stylesheet_url );
}

/**
* Get the action URL for the form element.
*
* @todo De-duplicate with \AMP_Form_Sanitizer::get_action_url().
*
* @param string $stylesheet_url Stylesheet URL.
* @return string|WP_Error Stylesheet URL.
*/
protected function normalize_stylesheet_url( $stylesheet_url ) {
if ( ! $stylesheet_url ) {
return new WP_Error( 'empty_stylesheet_url', __( 'Empty stylesheet URL', 'amp' ) );
}

$parsed_url = wp_parse_url( $stylesheet_url );
if ( ! $parsed_url ) {
return new WP_Error( 'stylesheet_url_parse_error', __( 'Stylesheet URL parse error', 'amp' ) );
}

// If a scheme was provided, there's nothing to do.
if ( ! empty( $parsed_url['scheme'] ) ) {
return $stylesheet_url;
}

$parsed_home_url = wp_parse_url( home_url() );

// Supply the same scheme as the site.
$parsed_url['scheme'] = $parsed_home_url['scheme'];

// Set an empty path if none is defined but there is a host.
if ( ! isset( $parsed_url['path'] ) && isset( $parsed_url['host'] ) ) {
$parsed_url['path'] = '';
}

if ( ! isset( $parsed_url['host'] ) ) {
$parsed_url['host'] = $parsed_home_url['host'];
}

if ( ! isset( $parsed_url['path'] ) ) {
// If there is action URL path, use the one from the request.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ); // @todo This is wrong because it includes the path.
} elseif ( '' !== $parsed_url['path'] && '/' !== $parsed_url['path'][0] ) {
// If the path is relative, append it to the current request path.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ) . trailingslashit( $parsed_url['path'] ); // @todo This is wrong because it includes the path.
}

// Rebuild the URL.
$stylesheet_url = $parsed_url['scheme'] . '://';
if ( isset( $parsed_url['user'] ) ) {
$stylesheet_url .= $parsed_url['user'];
if ( isset( $parsed_url['pass'] ) ) {
$stylesheet_url .= ':' . $parsed_url['pass'];
}
$stylesheet_url .= '@';
}
$stylesheet_url .= $parsed_url['host'];
if ( isset( $parsed_url['port'] ) ) {
$stylesheet_url .= ':' . $parsed_url['port'];
}
$stylesheet_url .= $parsed_url['path'];
if ( isset( $parsed_url['query'] ) ) {
$stylesheet_url .= '?' . $parsed_url['query'];
}
if ( isset( $parsed_url['fragment'] ) ) {
$stylesheet_url .= '#' . $parsed_url['fragment'];
}

return esc_url_raw( $stylesheet_url );
}

/**
* Fetch external stylesheet.
*
Expand All @@ -1427,9 +1496,9 @@ private function get_stylesheet_from_url( $stylesheet_url ) {
*/
private function fetch_external_stylesheet( $url ) {

// Prepend schemeless stylesheet URL with the same URL scheme as the current site.
if ( '//' === substr( $url, 0, 2 ) ) {
$url = wp_parse_url( home_url(), PHP_URL_SCHEME ) . ':' . $url;
$url = $this->normalize_stylesheet_url( $url );
if ( is_wp_error( $url ) ) {
return $url;
}

try {
Expand Down
9 changes: 6 additions & 3 deletions includes/validation/class-amp-validated-url-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1553,7 +1553,7 @@ public static function handle_validate_request() {
throw new Exception( 'invalid_post' );
}
if ( ! current_user_can( 'edit_post', $post->ID ) ) {
throw new Exception( 'unauthorized' );
throw new Exception( __( 'Unable to access validated URL screen.', 'amp' ) );
}
$url = self::get_url_from_post( $post );
} elseif ( isset( $_GET['url'] ) ) {
Expand All @@ -1562,8 +1562,8 @@ public static function handle_validate_request() {
throw new Exception( 'illegal_url' );
}
// Don't let non-admins create new amp_validated_url posts.
if ( ! current_user_can( 'manage_options' ) ) {
throw new Exception( 'unauthorized' );
if ( ! current_user_can( get_post_type_object( self::POST_TYPE_SLUG )->cap->edit_posts ) ) {
throw new Exception( __( 'Unable to access validated URL screen.', 'amp' ) );
}
}

Expand Down Expand Up @@ -1591,6 +1591,9 @@ public static function handle_validate_request() {
throw new Exception( AMP_Validation_Manager::get_validate_url_error_message( $stored->get_error_code(), $stored->get_error_message() ) );
}
$redirect = get_edit_post_link( $stored, 'raw' );
if ( empty( $redirect ) ) {
throw new Exception( __( 'Unable to access validated URL screen.', 'amp' ) );
}

$error_count = count(
array_filter(
Expand Down
2 changes: 1 addition & 1 deletion includes/validation/class-amp-validation-manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ public static function get_amp_validity_rest_field( $post_data, $field_name, $re
* @return boolean $has_cap Whether the current user has the capability.
*/
public static function has_cap() {
return current_user_can( 'edit_posts' );
return current_user_can( get_post_type_object( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG )->cap->edit_posts );
}

/**
Expand Down