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

QTS: fix to handle esc_sql() 4.8.3 breaking change #1157

Merged
merged 3 commits into from
May 2, 2022

Conversation

spleen1981
Copy link
Contributor

Since 4.8.3, ‘%’ characters will be replaced with a placeholder string with esc_sql().
This fix replaces back the placeholder to ‘%’ after esc_sql() call to retain the original behaviour.

Since 4.8.3, ‘%’ characters will be replaced with a placeholder string with esc_sql().
This fix replaces back the placeholder to ‘%’ after esc_sql() call to retain the original behaviour.
@herrvigg
Copy link
Collaborator

herrvigg commented May 1, 2022

Great if this fixes #1156! I trust you on this, I haven't a test case ready. Maybe @bagaweb could also test this before merge (easy to copy-paste the fix locally)?

Could you update the TODO in the readme if all is solved?

@@ -957,6 +957,7 @@ private function get_page_id_by_path( $page_path, $output = OBJECT, $post_type =
$page_path = str_replace( '%20', ' ', $page_path );
$parts = explode( '/', trim( $page_path, '/' ) );
$parts = array_map( 'esc_sql', $parts );
$parts = array_map( array( $wpdb, 'remove_placeholder_escape' ), $parts );
$parts = array_map( 'sanitize_title_for_query', $parts );
Copy link
Collaborator

Choose a reason for hiding this comment

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

The 3x array_map may be regrouped in one function (even anonymous) for faster execution in a sngle array_map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spleen1981
Copy link
Contributor Author

spleen1981 commented May 1, 2022

Could you update the TODO in the readme if all is solved?

This fixes only #1156 which is relevant to slugs.
TODO list points are relevant to permalink bases, that's a separate issue, I will take a look.

@bagaweb you can apply this patch for a quick test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants