WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#10860 closed defect (bug) (invalid)

wpdb::escape_by_ref() is broken

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.8.4
Component: General Keywords:
Focuses: Cc:

Description

Once upon a time there was established a function with parameters passed by reference to escape string data. The function was called escape_by_ref(). I assume this was done to save memory.

Now the function looks like this:

	/**
	 * Escapes content by reference for insertion into the database, for security
	 *
	 * @since 2.3.0
	 *
	 * @param string $s
	 */
	function escape_by_ref(&$string) {
		$string = $this->_real_escape( $string );
	}

Additionally escape_by_ref() is not a pair with escape(). Maybe that was sometime?

Change History (4)

comment:1 hakre5 years ago

  • Milestone Unassigned deleted
  • Version set to 2.8.4

comment:2 dd325 years ago

  • Milestone set to Unassigned

uhh... soo.. the issue is that wpdb::escape_by_ref() doesnt use wpdb::escape()?

Or something else entirely..

comment:3 hakre5 years ago

The question is wether or not such a function a) makes sense at all and b) if it should be there wether or not this is a pair to the ->escape() function.

The plus of escape_by_ref() over escape() is that it uses mysql_real_escape_string. ->escape() fails to properly escape data to be used with the databse "because it makes problems" (that comment isn't in there any longer but the escaping problem is). so escape_by_ref() does indeed what it is documented for.

so a) and b) should be answered, I can update the functions comments.

then I would like to know if this function was created to save some memory.

comment:4 nacin4 years ago

  • Milestone Unassigned deleted
  • Resolution set to invalid
  • Status changed from new to closed

Re-open with a clearer direction on the ticket. I'm really not sure what you mean by calling it broken.

We use escape_by_ref() in prepare(), as we use it as a callback in array_walk(), which requires by ref for what we're doing.

Note: See TracTickets for help on using tickets.