WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#35631 closed defect (bug) (fixed)

Possible PHP notice when updating Site Info

Reported by: flixos90 Owned by: jeremyfelt
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: good-first-bug has-patch
Focuses: multisite Cc:

Description

When updating a site's domain & path in the "Info" tab of Site Info, it can lead to a PHP notice being shown if a path is not included.
While paths are a crucial part to working with subdirectory installs, they are not really used on subdomain installs. In my opinion you should be able to enter http://example.com without appending the trailing slash, and WordPress should handle this automatically. Basically, if there's no trailing slash, assume the path is simply '/', otherwise take what's provided.

The notice is being generated because if you do not provide a path, the path key from parse_url() is not available. This is it:
Notice: Undefined index: path in /Volumes/HDD/user/site/web/core/wp-admin/network/site-info.php on line 83

Attachments (2)

35631.diff (772 bytes) - added by kjbenk 5 years ago.
This does not sound like a major issue but here is a patch to check for each part of the URL that is needed. If the URL is not valid it will tell the user.
35631.2.diff (662 bytes) - added by kjbenk 5 years ago.
Patch v2 to set the path? index to "/" if not set.

Download all attachments as: .zip

Change History (10)

@kjbenk
5 years ago

This does not sound like a major issue but here is a patch to check for each part of the URL that is needed. If the URL is not valid it will tell the user.

#1 @kjbenk
5 years ago

  • Keywords has-patch good-first-bug added

#2 follow-up: @flixos90
5 years ago

Thanks for the patch @kjbenk. However I'd rather have it detect whether the path is set, and otherwise simply define a path of only a trailing slash. I think showing an error message would confuse the user here.

Last edited 5 years ago by flixos90 (previous) (diff)

#3 in reply to: ↑ 2 ; follow-up: @kjbenk
5 years ago

Replying to flixos90:

I'd rather have it detect whether the path is set, and otherwise simply define a path of only a trailing slash. I think showing an error message would confuse the user here.

I am not 100% familiar with how the site url affects multisite but would this create any unwanted affects?

#4 in reply to: ↑ 3 @flixos90
5 years ago

Replying to kjbenk:

Replying to flixos90:

I'd rather have it detect whether the path is set, and otherwise simply define a path of only a trailing slash. I think showing an error message would confuse the user here.

I am not 100% familiar with how the site url affects multisite but would this create any unwanted affects?

It won't since basically, there should not be any difference in whether you specify a URL with or without trailing slash. However, every site needs to have a path (when using subdomains only, this path is '/') - so if you don't specify a path, we can safely assume it is the '/' I think.

#5 @flixos90
5 years ago

  • Keywords needs-patch added; has-patch removed

@kjbenk
5 years ago

Patch v2 to set the path? index to "/" if not set.

#6 @flixos90
5 years ago

  • Keywords has-patch added; needs-patch removed

@kjbenk Thanks, that looks good in my opinion!

#7 @jeremyfelt
5 years ago

  • Milestone changed from Awaiting Review to 4.5
  • Owner set to jeremyfelt
  • Status changed from new to reviewing

#8 @jeremyfelt
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 36561:

Multisite: Avoid a PHP Notice when saving a site address without a path.

Props kjbenk.
Fixes #35631.

Note: See TracTickets for help on using tickets.