Merge pull request #890 from Automattic/fix/get_validated_css_file_path

Detect amp_css_bad_file_extension prior to looking at filesystem
ampproject · Jan 26, 2018 · f877050 · f877050
2 parents 1987736 + efe6e95
commit f877050
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 7 deletions.
diff --git a/includes/class-amp-wp-styles.php b/includes/class-amp-wp-styles.php
@@ -60,10 +60,15 @@ public function get_validated_css_file_path( $src, $handle ) {
 
 		/** This filter is documented in wp-includes/class.wp-styles.php */
 		$src = apply_filters( 'style_loader_src', $src, $handle );
+		$src = esc_url_raw( $src );
 
 		// Strip query and fragment from URL.
-		$src = preg_replace( ':[\?#].+:', '', $src );
-		$src = esc_url_raw( $src );
+		$src = preg_replace( ':[\?#].*$:', '', $src );
+
+		if ( ! preg_match( '/\.(css|less|scss|sass)$/i', $src ) ) {
+			/* translators: %1$s is stylesheet handle, %2$s is stylesheet URL */
+			return new WP_Error( 'amp_css_bad_file_extension', sprintf( __( 'Skipped stylesheet %1$s which does not have recognized CSS file extension (%2$s).', 'amp' ), $handle, $src ) );
+		}
 
 		$includes_url = includes_url( '/' );
 		$content_url  = content_url( '/' );
@@ -77,11 +82,6 @@ public function get_validated_css_file_path( $src, $handle ) {
 			$css_path = ABSPATH . 'wp-admin' . substr( $src, strlen( $admin_url ) - 1 );
 		}
 
-		if ( ! preg_match( '/\.(css|less|scss|sass)$/i', $css_path ) ) {
-			/* translators: %1$s is stylesheet handle, %2$s is stylesheet URL */
-			return new WP_Error( 'amp_css_bad_file_extension', sprintf( __( 'Skipped stylesheet %1$s which does not have recognized CSS file extension (%2$s).', 'amp' ), $handle, $src ) );
-		}
-
 		if ( ! $css_path || false !== strpos( '../', $css_path ) || 0 !== validate_file( $css_path ) || ! file_exists( $css_path ) ) {
 			/* translators: %1$s is stylesheet handle, %2$s is stylesheet URL */
 			return new WP_Error( 'amp_css_path_not_found', sprintf( __( 'Unable to locate filesystem path for stylesheet %1$s (%2$s).', 'amp' ), $handle, $src ) );

diff --git a/tests/test-class-amp-wp-styles.php b/tests/test-class-amp-wp-styles.php
@@ -84,6 +84,10 @@ public function test_get_validated_css_file_path() {
 		$r = $wp_styles->get_validated_css_file_path( content_url( 'themes/twentyseventeen/404.css' ), 'bad' );
 		$this->assertInstanceOf( 'WP_Error', $r );
 		$this->assertEquals( 'amp_css_path_not_found', $r->get_error_code() );
+
+		$r = $wp_styles->get_validated_css_file_path( get_template_directory() . '/style.css', 'bad' );
+		$this->assertInstanceOf( 'WP_Error', $r );
+		$this->assertEquals( 'amp_css_path_not_found', $r->get_error_code() );
 	}
 
 	/**