#11643 closed defect (bug) (fixed)
Invalid code in wp-login.php
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | General | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
This just does not makes any sense:
if ( !$secure_cookie && whatever() && ... ) secure_cookie = false;
Found in line 480-481 in wp-login.php while reviewing #9207. Fix is simple, will attach a patch.
Attachments (1)
Change History (9)
#2
@
15 years ago
- Keywords needs-testing added; tested removed
Isn't it there in order to allow for a non-ssl admin while enforcing ssl on the login form?
Looks like it.. But its not doing its job if thats the case. I'm wondering if that was supposed to set it to true myself.
hakre: Can you please explain under what conditions patches have been tested before adding the tested keyword?
needs-testing: Need to verify all the cases are covered with regard to Login https...
#3
follow-up:
↓ 4
@
15 years ago
If I put testing there it at least means that the code was executed with the patch applied.
For these SSL related tickets I'm additionally working on a special secure wordpress setup with it's own confgiguration. I keep that on trunk plus the one or other patch all the time. When done I hopefully can finish the documentation properly as well.
#4
in reply to:
↑ 3
@
15 years ago
Replying to hakre:
If I put testing there it at least means that the code was executed with the patch applied.
That makes the tested keyword pretty much useless.
What that line is saying is that if the user was redirected to an ssl login page from a non ssl admin link and that secure login is required but secure admin is not, then don't require a secure cookie. This way the user can POST their login creds over https but not be forced to visit the admin via https. Yes, some people want this. You can do this in gmail too, for example. Also, there is a difference between $secure_cookie = false and $secure_cookie = .
From what I see, the code does what is intended and removing that line breaks a currently supported scenario that is widely used on wordpress.com, for one.
A comment describing all of that is much needed though, as the code is impenetrable. I'll add something.
#7
@
15 years ago
That doesnt really explain the line anymore...
if ( var is false &.... then set var to false
It'll never be run if $secure_cookie is true, or for that matter, anything non-null/non-false/non-empty.
If $secure_cookie is true, Then none of the rest will be run, and it'll never be set to false. If $secure_cookie is false, then it may get set to false again.
I can see what the logic is intending, I just dont see how it can ever work with !$secure_cookie at the start.
Isn't it there in order to allow for a non-ssl admin while enforcing ssl on the login form?