Make WordPress Core

Opened 13 years ago

Closed 11 years ago

#16756 closed enhancement (fixed)

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

Reported by: hakre's profile hakre Owned by: nacin's profile 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 13 years ago.
Use of get_blog_prefix()
16756.2.patch (339 bytes) - added by hakre 13 years ago.
Fix in documentation

Download all attachments as: .zip

Change History (19)

@hakre
13 years ago

Use of get_blog_prefix()

@hakre
13 years ago

Fix in documentation

#1 @hakre
13 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.

#2 @kawauso
13 years ago

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

#3 @hakre
13 years ago

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

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

#4 @hakre
13 years ago

Related: #16764

#5 @kawauso
13 years ago

  • Owner kawauso deleted

Could someone take the reviewing status off? Cheers.

#6 @Backie
13 years ago

  • Cc Backie added

#7 @nacin
13 years ago

  • Type changed from defect (bug) to enhancement

#8 @c3mdigital
11 years ago

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

#9 @ocean90
11 years ago

  • Keywords close removed

#10 @ocean90
11 years ago

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

#11 @SergeyBiryukov
11 years 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.

#13 follow-up: @nacin
11 years 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.

#14 in reply to: ↑ 13 @SergeyBiryukov
11 years 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.

#15 follow-up: @ryan
11 years ago

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

#16 in reply to: ↑ 15 @hakre
11 years ago

We have get_blog_prefix() used in 16756.patch

#17 @nacin
11 years 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.