WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 8 months ago

#10935 closed defect (bug) (fixed)

WP_Query and is_day() bug

Reported by: raliste Owned by: ryan
Milestone: 3.7 Priority: normal
Severity: normal Version: 2.8.4
Component: Canonical Keywords: has-patch needs-testing commit 3.7-early
Focuses: Cc:

Description

When you configure Wordpress with permalinks such as /%year%/%month% and even /%year%/%month%/%day/ there is
a failure when you request URLs like /2009/10/58, because it's still generating the query to the database
(AND YEAR(wp_posts.post_date)='2009' AND MONTH(wp_posts.post_date)='10' AND DAYOFMONTH(wp_posts.post_date)='58').
Also, is_day() returns true, when it should be returning false.

As adding a post with that date is nearly impossible trough the wordpress admin or the database, no posts will be found, so
Wordpress will show "No page found", but with HTTP status 200 OK, not 404 Not Found.

I think it's important to validate the day on the applicacion side, so for some ideas to work correctly under WP
(like take advantage of sticky posts and show all post for the month in day requests), and most importantly to save
those wasted mysql queries.

Of course I could validate the day using a filter, but that is not the general idea!

Attachments (9)

daybug.jpg (163.5 KB) - added by raliste 5 years ago.
doing query!
canonical-bad-dates.diff (2.9 KB) - added by wonderboymusic 20 months ago.
10935.diff (2.0 KB) - added by wonderboymusic 16 months ago.
10935.tests.diff (1.4 KB) - added by kovshenin 14 months ago.
10935.2.diff (2.0 KB) - added by DrewAPicture 12 months ago.
10935.3.diff (2.2 KB) - added by DrewAPicture 12 months ago.
wp_checkdate
10935.4.diff (2.2 KB) - added by SergeyBiryukov 12 months ago.
10935.5.diff (3.5 KB) - added by wonderboymusic 8 months ago.
10935.6.diff (5.1 KB) - added by wonderboymusic 8 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 raliste5 years ago

Ok, I noticed that HTTP status is working, but still is_day() returns true.

raliste5 years ago

doing query!

comment:2 scribu5 years ago

  • Keywords needs-patch added
  • Milestone set to Future Release
  • Severity changed from major to normal

comment:3 westi5 years ago

  • Keywords reporter-feedback added

is_day() == true means that the current query has a day specified so should return true in this case.

I'm not sure I see a bug here.

comment:4 raliste5 years ago

You are right, but the actual bug is in doing the query to the database. I thought that if you validate the day before, you could save those wasted queries. Also, months never have more than 31 days, to querying for day 58 should obviously return false, because it's not a day!

comment:5 jane3 years ago

  • Keywords reporter-feedback removed

comment:6 solarissmoke3 years ago

But is_day() doesn't return true. WP_Query::set_404() makes sure it doesn't.

comment:7 wonderboymusic20 months ago

  • Component changed from Query to Canonical
  • Keywords has-patch added; needs-patch removed

If you pass an invalid date to WP_Query, it will fire a query that will fail and then 404. My patch 404s before the query is made and does canonical redirection on bad date URLs.

If it's a day url, and the day doesn't jive with the month: it will redirect to the month. If it's a month url, and the monthnum is 15 or something: it will redirect to the year.

Adds 2 functions: wp_is_valid_day() and wp_is_valid_month()

comment:8 wonderboymusic20 months ago

Looky there, PHP has a function called checkdate() which validates day / month / year and takes leap years into account. Boom: http://php.net/manual/en/function.checkdate.php

Patch updated.

wonderboymusic16 months ago

comment:9 wonderboymusic16 months ago

  • Milestone changed from Future Release to 3.6

Patch refreshed against trunk, ditches the new functions, just uses checkdate() - passes all Unit Tests

comment:10 nacin15 months ago

As of 3.5 we have a wp_checkdate() with a filter, I think mostly to support other calendars. This looks great at a glance, otherwise.

comment:11 SergeyBiryukov14 months ago

  • Keywords needs-unit-tests added

kovshenin14 months ago

comment:12 kovshenin14 months ago

  • Cc kovshenin added
  • Keywords needs-unit-tests removed

10935.diff looks sane. 10935.tests.diff adds some unit tests.

DrewAPicture12 months ago

comment:13 DrewAPicture12 months ago

  • Keywords needs-testing added

10935.2.diff fixes a small typo in the previous patch.

comment:14 SergeyBiryukov12 months ago

We should probably use wp_checkdate() (introduced in [21922] for #17180), as noted in comment:10.

comment:15 DrewAPicture12 months ago

10935.3.diff uses wp_checkdate(). A concatenated date of the format YYYY-MM-DD is passed in as $source_date.

DrewAPicture12 months ago

wp_checkdate

comment:16 SergeyBiryukov12 months ago

  • Keywords commit added

10935.4.diff assigns some variables for better readability.

SergeyBiryukov12 months ago

comment:17 ryan12 months ago

  • Keywords 3.7-early added

Punted per bug scrub. It is too late in 3.6 for permalink changes. Marking for 3.7-early.

comment:18 ryan12 months ago

  • Milestone changed from 3.6 to Future Release

comment:19 wonderboymusic9 months ago

  • Milestone changed from Future Release to 3.7

comment:20 wonderboymusic9 months ago

these are all marked 3.7-early

wonderboymusic8 months ago

wonderboymusic8 months ago

comment:21 wonderboymusic8 months ago

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

In 25280:

Check bad dates and redirect, instead of 404ing, as necessary and appropriate.
Adds query, conditional, and canonical Unit Tests.

Props kovshenin, SergeyBiryukov, DrewAPicture.
Fixes #10935.

Note: See TracTickets for help on using tickets.