View Issue Details

IDProjectCategoryView StatusLast Update
0005895Simple:Presscode - filterspublic2018-03-23 19:59
ReporterYellow SwordfishAssigned ToYellow Swordfish 
PrioritynormalSeverityN/AReproducibilityhave not tried
Status closedResolutionfixed 
Product Version5.7.5.2 
Target Version5.8Fixed in Version5.8 
Summary0005895: function esc_sql() not set up to allow for an array
DescriptionI had this error appear:

Error log wrote

file: /simple-press/sp-api/sp-api-class-spcfilters.php
line: 63
function: mysqli_real_escape_string
Warning | mysqli_real_escape_string() expects parameter 2 to be string, array given

I can only find two places after a quick search, where an array may be passed to this function:
  1. Class spcOptions - method update
  2. Function spa_save_permissions_edit_role() ($new_auths)

However - I see no reason why either of these should have been called during my short visit to my local dev system so there may be another lurking. Either way - allowing for an array of values seems like a sensible approach.
TagsNo tags attached.
change_log_textadapt function esc_sql() to allow for an array


Mr Papa

Mr Papa

2018-02-18 15:31

administrator   ~0019792

its never supported an array... its always just called the php function mysqli_real_escape_string()...

clearly an error in the locations that were calling it... and been that way for ages...

before we change to accept arrays (if that is the desire), we need to understand the impact to such a low level primitive that is called many, many times per page load....
Yellow Swordfish

Yellow Swordfish

2018-02-18 16:14

administrator   ~0019793

It is correct it has always been this way but the function was only introduced with version - the most recent - so it is very new. i don't have an older version to hand at the moment to check back what we did before that.

I don't see it would be any disaster to check if the data being provided was an array or not. It seems to be used some 65 or so times but a lot of those are in rarely used functions.
Mr Papa

Mr Papa

2018-02-18 18:17

administrator   ~0019794

before, we called a wp function which did the same... but they added some extra security stuff (strictly for wpdb->prepare) which broke things for a lot of folks therefore we rolled our own version to do what wp used to do... so it never really has supported arrays...

not arguing against making it work for arrays, but should do it with eyes wide open on performance and also make sure its needed from the callers (possibly like meta, just easier/better for caller... but dont know without research...


2018-02-20 09:58

administrator   ~0019795

Changeset [15640] by andy on 2018-02-20 04:58:58 -0500 (Tue, 20 Feb 2018)

test issue 0005895
The version allows for an array argument to be sent to the esc_sql function

 Changed Files:

U trunk/sp-api/sp-api-class-spcfilters.php

Yellow Swordfish

Yellow Swordfish

2018-02-20 10:00

administrator   ~0019796

Take a look...
The alternative is to track down the code that sends an array argument and change all of those. as I said - I found two but there has to be another one I missed.

Issue History

Date Modified Username Field Change
2018-02-18 11:23 Yellow Swordfish New Issue
2018-02-18 11:24 Yellow Swordfish Description Updated View Revisions
2018-02-18 15:31 Mr Papa Note Added: 0019792
2018-02-18 16:14 Yellow Swordfish Note Added: 0019793
2018-02-18 18:17 Mr Papa Note Added: 0019794
2018-02-20 09:58 svn =>
2018-02-20 09:58 svn Note Added: 0019795
2018-02-20 09:58 svn Status new => testing
2018-02-20 10:00 Yellow Swordfish Note Added: 0019796
2018-02-21 01:45 Mr Papa Assigned To => Yellow Swordfish
2018-02-21 01:45 Mr Papa Status testing => assigned
2018-02-21 01:45 Mr Papa Status assigned => testing
2018-03-23 19:59 Mr Papa Status testing => closed
2018-03-23 19:59 Mr Papa Resolution open => fixed
2018-03-23 19:59 Mr Papa Fixed in Version => 5.8
2018-03-23 19:59 Mr Papa change_log_text function esc_sql() not set up to allow for an array => adapt function esc_sql() to allow for an array