Make WordPress Core

Opened 4 years ago

Last modified 4 weeks ago

#53355 reviewing defect (bug)

wp-signup.php with `new` in the query string results in a "site does not exist" message even if the site does exist

Reported by: henrywright's profile henry.wright Owned by: audrasjb's profile audrasjb
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch has-unit-tests dev-feedback has-test-info
Focuses: multisite Cc:

Description

Steps to reproduce:

  1. Visit example.com/wp-signup.php?new=sport where sport is the address of an existing site in your network. Note I'm using sport as an example. You should replace sport with the address of an existing blog in your network.

You will get the following message appear on the page:

The site you were looking for, https://sport.example.com/, does not exist, but you can create it now!

Attachments (4)

53355.diff (1.6 KB) - added by henry.wright 4 years ago.
53355.2.diff (1.8 KB) - added by henry.wright 4 years ago.
53355.3.diff (1.8 KB) - added by henry.wright 4 years ago.
53355.4.diff (1.9 KB) - added by henry.wright 4 years ago.

Download all attachments as: .zip

Change History (25)

#1 @henry.wright
4 years ago

To add, the network install I used to test allows both user and blog registrations

@henry.wright
4 years ago

#2 @henry.wright
4 years ago

  • Keywords has-patch added

53355.diff uses domain_exists() to check if a site name is already taken before outputting the message.

@henry.wright
4 years ago

#3 @henry.wright
4 years ago

53355.2.diff fixes the value of $domain which is passed to domain_exists() as the first param.

@henry.wright
4 years ago

#4 @henry.wright
4 years ago

53355.3.diff ensures the right parts of the extracted host name are used to build the domain value.

#5 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.9
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Related: #53348

@henry.wright
4 years ago

#6 @henry.wright
4 years ago

53355.4.diff adds a separation character when building the domain value

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


4 years ago

#9 @audrasjb
4 years ago

  • Keywords needs-testing needs-refresh added
  • Milestone changed from 5.9 to 6.0

As per today's bug scrub and since this ticket needs to be refreshed against trunk and properly tested, let's move it to next milestone.

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


3 years ago

#11 @kirasong
3 years ago

  • Keywords needs-unit-tests added; needs-refresh removed

Checked into this ticket in a bug scrub today, and here's a synopsis of the suggestions to move things forward:

Looks like the patch applies at the moment, so removing needs-refresh.

Tests would be helpful, so adding needs-unit-tests as well.

Last edited 3 years ago by kirasong (previous) (diff)

This ticket was mentioned in PR #2655 on WordPress/wordpress-develop by felipeelia.


3 years ago
#12

  • Keywords has-unit-tests added; needs-unit-tests removed

This PR aims to fix the problem outlined in ticket 53355 but also validates the field right away, so if a name of an existing blog is sent via the query string, the user will see an error in the first page load.

Trac ticket: https://core.trac.wordpress.org/ticket/53355

#13 @costdev
3 years ago

  • Milestone changed from 6.0 to 6.1

With 6.0 RC1 tomorrow and the patch still needing testing, I'm moving this ticket to the 6.1 milestone.

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


3 years ago

#15 @audrasjb
3 years ago

  • Milestone changed from 6.1 to Future Release

Unfortunately, this ticket didn’t get more momentum in 6.1 than in 6.0… sorry about that, but the proposed patch still needs testing, so let's move it to Future Release milestone, at least until it's properly reviewed by a Network component maintainer.

This ticket was mentioned in Slack in #core-multisite by realloc. View the logs.


5 weeks ago

This ticket was mentioned in PR #8890 on WordPress/wordpress-develop by @SirLouen.


5 weeks ago
#17

This is a refresh of #2655

Code still needs to be reviewed.

Trac ticket: https://core.trac.wordpress.org/ticket/53355

#18 @SirLouen
5 weeks ago

  • Keywords dev-feedback added; needs-testing removed

Combined Bug Reproduction and Patch Test Report

Description

✅ This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8890.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty 2.9
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Testing Instructions

  • Following instructions from OP
  1. Set up a Multisite
  2. Create a new blog, call it for example, "sport"
  3. Go to Settings > Network Settings and set for example "User accounts may be registered"
  4. Go to wp-signup.php?new=sport
  5. 🐞 The error appears: The site you were looking for, http://localhost:8889/sport/, does not exist.

Expected Results

  • Nothing should appear as the blog was already created

Actual Results

  1. ✅ Issue resolved with patch.
  2. ✅ Unit Tests are adequate, not passing pre-patch and passing post-patch as expected

Additional Notes

  • I have not reviewed this code. The code was not applying but I found that making it apply was affordable, so I have integrated it just for testing purposes. From a bird's-eye view, it looks adequate, probably ready to hook up into the next release.
Last edited 4 weeks ago by SirLouen (previous) (diff)

#19 @SirLouen
5 weeks ago

  • Keywords has-test-info added

#20 @sandeepdahiya
4 weeks ago

Bug Reproduction and Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: Patch-8890.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Firefox 139.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. Bug Reproduction: The issue was successfully reproduced by following the steps outlined by the original poster. The bug still persists.When a user attempts to register a site that is already taken—by navigating to wp-signup.php?new=sport—WordPress still displays the message:"The site you were looking for, https://sport.localhost:8889/, does not exist, but you can create it now!"This message appears at the bottom of the page, below the "Create Site" button, even though the site is already registered. This is the screenshot before the patch.
  1. Patch report: The issue resolved with patch as expected. It doesn't display the message as mentioned above and when we try to create site using the above slug. It says "Sorry, the site is reserved". This is the screenshot after the patch.

Additional Notes

  1. screenshot before the patch
  1. screenshot after the patch

#21 @audrasjb
4 weeks ago

  • Owner changed from SergeyBiryukov to audrasjb

Thank you both for testing this patch. Reassigning for code review.

Note: See TracTickets for help on using tickets.