Opened 14 years ago
Closed 11 years ago
#16884 closed defect (bug) (fixed)
Wrong order of parameters in str_replace() in setup-config's get_bloginfo()
Reported by: | hakre | Owned by: | dd32 |
---|---|---|---|
Milestone: | 3.7 | Priority: | normal |
Severity: | normal | Version: | 3.1 |
Component: | Upgrade/Install | Keywords: | has-patch dev-feedback |
Focuses: | Cc: |
Description
See str_replace()
Replacing some string with something in an empty string is obviously bogus.
Attachments (5)
Change History (25)
#1
@
14 years ago
It looks more like someone typed the arguments in the wrong order when trying to make setup-config.php install in a subdir safe.
So we should test that and fix that if it is the real bug.
#2
@
14 years ago
Fix if the first parameter was mixed with the third one and the second one with the first one and the third one with the first one.
#3
follow-up:
↓ 17
@
14 years ago
Related: [13232]; #12159; 12159.4.diff ;
As the change was introduced 13 month ago and no error has been reported about it, I would furthermost assume that the replacement is not of a vital need. More than a year of testing incl. with live sites is a pretty long time IMHO so from usage data a replacement is not needed.
Anyway the change has been performed by nacin and he probably remembers why this went in so he can actually clarify for what that str_replace() call was needed.
#6
follow-up:
↓ 7
@
14 years ago
that change was to allow the HTTP API to be included, which by default, calls get_bloginfo() for the default user agent.
The result of this is that the API request has always set it's user agent to http://host/ instead of http://host/some/directory/. For all intents this hasn't caused an issue as the API doesn't require the URL for the salt generation.
For what does that /#@+ stand for?
It's a special PHPDoc syntax which basically means, Ignore the following functions in the PHPDoc / XRef
#7
in reply to:
↑ 6
@
14 years ago
Replying to dd32:
that change was to allow the HTTP API to be included, which by default, calls get_bloginfo() for the default user agent.
The result of this is that the API request has always set it's user agent to http://host/ instead of http://host/some/directory/. For all intents this hasn't caused an issue as the API doesn't require the URL for the salt generation.
Thanks for the information. Then the replacement was not needed at all I assume.
For what does that /#@+ stand for?
It's a special PHPDoc syntax which basically means, Ignore the following functions in the PHPDoc / XRef
I first thought it would be PHPDoc syntax but could not find any information about it. @ignore is enough for the next element (the function) IMHO.
But I was not thinking about PHPXRef for which it then is needed only? Or is this really a PHPDoc feature?
#9
@
14 years ago
The purpose of the str_replace() would've been to guess the real installation URL, and provide a consistent return value between the stub and the real function.
While it makes no practical difference what the function returns, It might as well return the correct guessed url, therefor, 16884.2.patch is probably the correct option here.
#10
@
14 years ago
If blind, second patch is safest bet.
I'm just learning this:
- In case no
/wp-config.php
file exists (e.g. before installation or on error), the user is motivated to request/wp-admin/setup-config.php
. - This means that
/wp-admin/setup-config.php
is called on installing the software. - There is no other place in core that links nor includes
/wp-admin/setup-config.php
.
So by usage, $_SERVER['PHP_SELF']
is most probably /wp-admin/setup-config.php
, at least if it contains the default value (PHP Manual).
#11
@
14 years ago
- The function is defined to be called as
get_bloginfo( 'url' )
later on in WP_Http::request(). get_bloginfo( 'url' )
is the same ashome_url()
home_url()
isget_home_url(null, '')
get_home_url(null, '')
is basicallyget_option( 'home' )
.
That option does not contain the trailing slash.
#12
@
14 years ago
Third patch is wrong compared with default get_bloginfo( 'url' )
behavior, but should not pose any problems. Same for the first patch.
#13
follow-up:
↓ 14
@
14 years ago
It's a parameter order issue. I did subject, match, replace, instead of match, replace, subject.
The phpdoc allows multiple declarations to all take the same phpdoc. I'm not sure it's necessary here as it is one function.
#14
in reply to:
↑ 13
@
14 years ago
Replying to nacin:
It's a parameter order issue. I did subject, match, replace, instead of match, replace, subject.
Thanks for clarification.
The phpdoc allows multiple declarations to all take the same phpdoc. I'm not sure it's necessary here as it is one function.
Ah, cool.
As the parameter issue could be clarified, this clarified the use of the str_replace statement as well.
I created another patch that is making use of the wp_guess_url() function as it is used to populate get_option( 'home' )
on install. It's available through /wp-includes/functions.php
(thx t31os).
#16
@
14 years ago
- Summary changed from str_replace() in setup-config's get_bloginfo() is bogus to URL generation in setup-config's get_bloginfo()
#17
in reply to:
↑ 3
@
12 years ago
- Component changed from General to Upgrade/Install
- Milestone changed from Awaiting Review to Future Release
#18
@
11 years ago
- Summary changed from URL generation in setup-config's get_bloginfo() to Wrong order of parameters in str_replace() in setup-config's get_bloginfo()
removal of bogus code