WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 10 months ago

#16756 closed enhancement (fixed)

PHP5-port - Access Violations on wpdb::$prefix

Reported by: hakre Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.1
Component: Database Keywords: has-patch
Focuses: Cc:

Description

The to be accessed privately member wpdb::$prefix is accessed publicly in multiple places.

This would do fatal errors if the property would be ported to PHP 5 with the documented behavior.

The errorneous usage in the places in question might reveal that implementation details were missed because the prefix can differ on multi-site setups which is not technically guaranteed when accessing the private member.

The private member was only used "read-only" inside the class. Probably this is a documentation issue. It's set set_prefix() and set_blog_id() functions (only) and always to the result of wpdb::get_blog_prefix().

Attachments (2)

16756.patch (3.9 KB) - added by hakre 3 years ago.
Use of get_blog_prefix()
16756.2.patch (339 bytes) - added by hakre 3 years ago.
Fix in documentation

Download all attachments as: .zip

Change History (19)

hakre3 years ago

Use of get_blog_prefix()

hakre3 years ago

Fix in documentation

comment:1 hakre3 years ago

The second one is just for the case that it might be argued that this is issue with documentation (instead with the code). However I do not recommend to use it.

comment:2 kawauso3 years ago

  • Cc otterish@… added
  • Owner set to kawauso
  • Status changed from new to reviewing

comment:3 hakre3 years ago

Related: #16762 (for wpdb::$base_prefix)

Last edited 3 years ago by hakre (previous) (diff)

comment:4 hakre3 years ago

Related: #16764

comment:5 kawauso3 years ago

  • Owner kawauso deleted

Could someone take the reviewing status off? Cheers.

comment:6 Backie3 years ago

  • Cc Backie added

comment:7 nacin3 years ago

  • Type changed from defect (bug) to enhancement

comment:8 c3mdigital11 months ago

  • Keywords close added; has-patch removed
  • Resolution set to maybelater
  • Status changed from reviewing to closed

comment:9 ocean9011 months ago

  • Keywords close removed

comment:10 ocean9011 months ago

  • Component changed from General to Database
  • Milestone Awaiting Review deleted

comment:11 SergeyBiryukov11 months ago

  • Keywords has-patch added
  • Milestone set to 3.7
  • Resolution maybelater deleted
  • Status changed from closed to reopened

16756.2.patch looks like a valid inline documentation fix to me.

comment:13 follow-up: nacin11 months ago

We also have $wpdb->get_blog_prefix() (with no arguments). Maybe all usage of $wpdb->prefix should start wrapping that function — potentially even having it set via a magic getter. (They essentially are already linked, any methods that change the prefix sets $wpdb->prefix.)

At that point, should $wpdb->prefix even be recommended? I'd wonder if keeping it as @access private (as in, "please don't use it") is OK.

I dunno, maybe ryan has an opinion as the two of us shaped a lot of this back in 3.0.

comment:14 in reply to: ↑ 13 SergeyBiryukov11 months ago

Replying to nacin:

We also have $wpdb->get_blog_prefix() (with no arguments). Maybe all usage of $wpdb->prefix should start wrapping that function — potentially even having it set via a magic getter.

Then maybe 16756.patch is the way to go.

comment:15 follow-up: ryan11 months ago

We have get_blog_prefix(). Might as well use it.

comment:16 in reply to: ↑ 15 hakre10 months ago

We have get_blog_prefix() used in 16756.patch

comment:17 nacin10 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from reopened to closed

In 25615:

Use the wpdb method instead of $wpdb->prefix.

props hakre.
fixes #16756.

Note: See TracTickets for help on using tickets.