WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#26984 closed defect (bug) (invalid)

inconsistency between get_option() and get_site_option() for siteurl

Reported by: Denis-de-Bernardy Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.9
Component: Security Keywords:
Focuses: multisite Cc:

Description

When creating a network, get_site_option('siteurl') gets defined with a trailing slash.

Seeing that it's only ever used to define the cookie hash, is there any particular reason for this, or is this just cruft from before the multisite merge?

Change History (4)

#1 follow-up: @jeremyfelt
7 years ago

  • Component changed from General to Permalinks
  • Focuses multisite added

The additional / was introduced in [13760] specifically to "force users to re-login after installing network" in an attempt to fix #12142.

The issue continued to progress, so I'm not entirely sure that was the absolute solution.

@nacin mentioned in #12142:18 that this method "should be replaced with wp_logout() or wp_clear_auth_cookie(), and we need to investigate whether A) there is a looping issue with MS, and B) we can invalidate cookies if we can detect new keys."

I'm not sure if it hurts anything now or if it would hurt anything if we took it out now that it's been there for 4 years.

#2 in reply to: ↑ 1 @Denis-de-Bernardy
7 years ago

Replying to jeremyfelt:

The additional / was introduced in [13760] specifically to "force users to re-login after installing network" in an attempt to fix #12142.

More like sweeping #12142 under a rug without fixing anything...

@nacin mentioned in #12142:18 that this method "should be replaced with wp_logout() or wp_clear_auth_cookie(), and we need to investigate whether A) there is a looping issue with MS, and B) we can invalidate cookies if we can detect new keys."

I only scanned that ticket. How would one reproduce that issue to check if it's still occurring? It sounded like it would only ever happen when WP introduces new auth/salt site options (and defines) during an upgrade.

#3 @Denis-de-Bernardy
7 years ago

  • Component changed from Permalinks to Security

#4 @nacin
7 years ago

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

#12142 brings back nightmares. And yes, it did "fix" it. You're the first person to say something in four years, and it wasn't about the redirect loops that were painful to debug. So painful that we "fixed" it this way. It wasn't salts/keys, best I can remember.

I don't know how to reproduce it, but I'm closing this ticket out, it's invalid. If you wish to open a new ticket on improving our handling of this, that's fine — but please come armed with steps to reproduce and a patch.

Note: See TracTickets for help on using tickets.