Make WordPress Core

Opened 14 years ago

Closed 23 months ago

#11159 closed defect (bug) (invalid)

WP_SITEURL and bloginfo('siteurl') inconsistent, or WP_SITEURL should be defined

Reported by: anmari's profile anmari Owned by:
Milestone: Priority: normal
Severity: minor Version:
Component: General Keywords: has-unit-tests
Focuses: Cc:

Description

The wordpress directory constant WP_SITEURL remains undefined if not defined in wp-config.php , although other constants referred to in http://codex.wordpress.org/Editing_wp-config.php do get defined in wp-settings.php. See from around line 109 and lines 228, 362, 370 etc onwards.

get_bloginfo('siteurl') defaults to get_bloginfo('home'). If a developer uses get_bloginfo('siteurl') and the wp install has wp_siteurl defined in the config file - the "wrong" url is returned from what the developer expects.

So while a developer can use other constants, they cannot reliably use WP_SITEURL. They must check for definition of WP_SITEURL and define WP_SITEURL themselves using 'wp_url', not 'siteurl', as that gives same as 'home', or always use bloginfo('wp-url') - slower.

See: general-template.php (in 2.9) lines 303 on has:

case 'url' :
case 'home' : DEPRECATED
case 'siteurl' :
DEPRECATED

$output = get_option('home');
break;

case 'wpurl' :

$output = get_option('siteurl');
break;

Surely
1) bloginfo('siteurl') even if deprecated, should call option('siteurl'), as per bloginfo('wp_url'), not 'home'
OR
2) WP_SITEURL (or WP_WPURL!!) should be defined in wp-settings if not defined in config

Attachments (3)

11159.patch (1.4 KB) - added by miyauchi 6 years ago.
Added tests for site_url() and WP_SITEURL
11159.2.patch (1.5 KB) - added by miyauchi 6 years ago.
11159.3.patch (1.5 KB) - added by miyauchi 6 years ago.

Download all attachments as: .zip

Change History (13)

#1 @scribu
14 years ago

  • Keywords needs-patch added
  • Milestone changed from Unassigned to Future Release

#2 @anmari
13 years ago

In wp 3.0.1 looks like default_constants.php should have :

if ( !defined('WP_SITEURL') )

define( 'WP_SITEURL', get_option('siteurl') );

either around line 40 or around line 71
to be consistent

#3 @nacin
13 years ago

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

WP_SITEURL and WP_HOME are short-circuit constants. By default they should not be defined, there's no reason to define them. They should NOT be used directly.

When defined, these trigger a filter on pre_option_home and pre_option_siteurl, to then force get_option() to return the constant's value.

#4 @jeffmikels
6 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

This ticket should be reopened. The constants and the get_option are inconsistent again.

@miyauchi
6 years ago

Added tests for site_url() and WP_SITEURL

#5 @miyauchi
6 years ago

  • Keywords has-patch added; needs-patch removed

I added tests like following.

/**
 * @runInSeparateProcess
 * @preserveGlobalState disabled
 */
function test_wp_siteurl_constant() {
	define( 'WP_SITEURL', 'https://example.com' );
	$this->assertEquals( WP_SITEURL, site_url() );
}

Then this test failed.

1) Tests_URL::test_wp_siteurl_constant
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'https://example.com'
+'http://example.com'

Is this intentional?

Last edited 6 years ago by miyauchi (previous) (diff)

@miyauchi
6 years ago

@miyauchi
6 years ago

#6 @miyauchi
6 years ago

Following tests is passed.

	/**
	 * @runInSeparateProcess
	 * @preserveGlobalState disabled
	 */
	function test_wp_home_url_constant() {
		define( 'WP_HOME', 'https://example.com' );
		$this->assertEquals( WP_HOME, home_url() );
	}

But following is failed.

	/**
	 * @runInSeparateProcess
	 * @preserveGlobalState disabled
	 */
	function test_wp_siteurl_constant() {
		define( 'WP_SITEURL', 'https://example.com' );
		$this->assertEquals( WP_SITEURL, site_url() );
	}

And also, following will be passed.

$this->assertEquals( WP_SITEURL, get_option( 'siteurl' ) );

#7 @SergeyBiryukov
5 years ago

  • Keywords has-unit-tests added; WP_SITEURL constants has-patch removed
  • Milestone set to Awaiting Review

This ticket was mentioned in Slack in #core by chaion07. View the logs.


23 months ago

This ticket was mentioned in Slack in #core by mike. View the logs.


23 months ago

#10 @mikeschroder
23 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

This ticket was discussed in a triage session today.

Consensus was that the opinion given in https://core.trac.wordpress.org/ticket/11159#comment:3 is still valid.

@miyauchi, I agree that the behavior is a bit confusing, but looking at the docs for site_url (https://developer.wordpress.org/reference/functions/site_url/), my understanding is that this is intentional.

Thanks so much for the tests!

I think the behavior is covered by the tests in https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/url.php, but if you think something is missed there, we can discuss adding additional tests to be sure everything is covered!

Given the above, I'm going to close this ticket again.
As always, feel free to comment back to continue discussion if needed, or this seems inappropriate.

Note: See TracTickets for help on using tickets.