WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#3944 closed defect (bug) (duplicate)

Entering an invalid post timestamp will be tolerated and results in bad view links

Reported by: erikwasser Owned by:
Milestone: 2.6 Priority: low
Severity: normal Version: 2.1.2
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

1) Goto you your blog and create a new post.

2) Enter a subject and some blog text (doesn't really matter)

3) Enter an invalid date like 2007-04-31 and save the post as unpublished article.

The error is:

a) No error message for the invalid date. It will be just saved.

b) If you view the article the link looks like this: http://www.foo.invalid/2007/05/01/bug/

This is the wrong date. And the view link will result (even as admin with the right so see unpublished articles) in a 404.

c) The database data is screwed too:

Here are the important 3 fields:

post_date 2007-04-31 17:09:36

Non existing data. But this could be a mysql bug.

post_date_gmt = 2007-05-01 16:09:36

Wrong date.

guid = http://www.epstacy.de/2007/05/01/bug/

And wrong link for the article.

Attachments (4)

admin-functions-php.diff (1.8 KB) - added by jhodgdon 7 years ago.
Fix to make sure all posts created/updated have valid post_date fields
link-template-php.diff (1.3 KB) - added by jhodgdon 7 years ago.
Fix to get_permalink that makes its date handling more consistent with the querying that will retrieve the post later
post-php.diff (1.7 KB) - added by JDTrower 6 years ago.
Fix to make sure all posts created/updated have valid post_date fields based on the patch for admin-functions.php.
post-php2.diff (1.9 KB) - added by JDTrower 6 years ago.
Fix to make sure all posts created/updated have valid post_date fields based on the patch for admin-functions.php and makes changes to time normalization.

Download all attachments as: .zip

Change History (23)

comment:1 foolswisdom7 years ago

  • Milestone set to 2.2

comment:2 jhodgdon7 years ago

I tried to test this in [5012].

When I followed the instructions, using an April 31st date, and saving (not publishing), when I went back to the screen, the date was set to May 1.

When I tried publishing, it also worked fine. But in this testing blog, I didn't have permalink structure using dates.

So I changed to a permalink structure using dates, the standard one:

/%year%/%monthnum%/%day%/%postname%/

The already-published article now has a permalink with a May 1 date in it, and as reported by the person above, the permalink is invalid (click on the article title and it goes to a "cannot find anything" page).

In the MySQL database, the post_date field is 2006-04-31 21:19:57, while post_date_GMT is OK (May 2nd, due to my time zone).

So, I have verified it is a real bug, and I will see what I can do about fixing it.

comment:3 jhodgdon7 years ago

Come to think of it, the real problem here is that the logic for making permalinks is not doing the same thing as the logic for searching for a post from a permalink.

Of course, given that the date is invalid, this may not be much of a surprise.

Then of course the other problem is that the time stamp is invalid and is saved in the MySQL database with an invalid date.

comment:4 jhodgdon7 years ago

OK.

I have a fix for admin-functions.php, the two functions where it creates and modifies saved MySQL data for a post. This will ensure that the date it saves is always valid. Will attach patch shortly.

If an invalid date gets into the database, get_permalink (which creates the permalink) actually will make it into a valid date when creating the permalink. It uses strtotime to convert the db entry to a timestamp, then the PHP date function to convert to month, day, year, etc.

The issue is that the query that goes and tries to find the post, given the permalink string, will query vs. the stored timestamp, and the MySQL functions used to query are not smart enough to realize it's not valid. They just return the stored pieces of the field. I do not know how to fix that. Maybe someone who understands the query logic better can fix it, if deemed necessary.

Hopefully, fixing it so that an invalid date is never stored will be enough... I'll attach a patch within a few minutes.

jhodgdon7 years ago

Fix to make sure all posts created/updated have valid post_date fields

comment:5 jhodgdon7 years ago

  • Keywords has-patch admin write post added

comment:6 jhodgdon7 years ago

One more note. It would be possible to modify get_permalink (the function that actually makes the permalink) so that instead of converting the DB information in the post_date field to a real date and then extracting parts of it, it would just extract the parts of the actual DB information directly.

That would mean that if somehow an invalid date got into the database (which it shouldn't any more, once the above patch is installed), the permalink generated by get_permalink would at least be findable by the query logic, since it would match the actual database info used by the query logic.

Thoughts? This would not be a hard fix to do, but I am not sure it is worth doing to take care of something that should no longer happen. At least if the admin-functions route is the only way posts are created or updated. ???

comment:7 jhodgdon7 years ago

OK, I tested this idea of patching get_permalink.

a) Reverted admin_functions.php to version [5007] (without the above patch), so that I could create a post_date that was invalid.

b) Created a post with an invalid date (April 31). The permalink doesn't work (as described above), as the permalink has a date of May 1, and the database has April 31, so when you click the permalink, it cannot find the post in the database.

c) Applied a new patch (coming soon) to the get_permalink function to use the database-stored month/day/year instead of converting to a valid date/time first.

d) Now the permalink says April 31, and it works! Permalinks on posts with valid dates are not affected.

So... I am not sure whether I would recommend installing this patch or not, but it does work, and should match what the querying logic is doing more closely.

It does lead to some odd behavior. Such as that now I have an archive for April 2006 in my monthly archives list on the sidebar. When I click on that archive, it does show my post (which is good), but the title of the archive says "Archive for May 2006", and the date of the post (displayed with the_date) says May 1, 2006.

Then again, it is an invalid date, so we would expect some things to be broken. But at least the permalinks are clickable (i.e. consistent with the query).

And I don't think it will break anyone's existing permalinks, because permalinks that were previously inconsistent with the query structure would have been broken anyway.

Well, I'll submit the patch and let you think about whether to install it or not.

jhodgdon7 years ago

Fix to get_permalink that makes its date handling more consistent with the querying that will retrieve the post later

comment:8 majelbstoat7 years ago

Confirming that admin-functions.diff ensured consistent dates throughout the database for me. Not sure if the changes to get_permalink are worth it with the first patch applied. A regular expression seems a bit overkill if we've guaranteed that the existing method will work.

comment:9 foolswisdom7 years ago

  • Milestone changed from 2.2 to 2.3

comment:10 f00f7 years ago

The patched admin-functions.php looks good.

But, please also patch that crappy time normalization that happens just above the current patch.

Replace

$hh = ($hh > 23 ) ? $hh -24 : $hh;
$mn = ($mn > 59 ) ? $mn -60 : $mn;
$ss = ($ss > 59 ) ? $ss -60 : $ss;

with

$hh %= 24;
$mn %= 60;
$ss %= 60;

This it will also work if s.o. enters 120 as the value for seconds.

Sorry for not supplying a patch for that.

comment:11 foolswisdom7 years ago

  • Keywords early added
  • Milestone changed from 2.3 to 2.4

comment:12 JDTrower6 years ago

I confirmed that the bug listed in this ticket was valid, however I got slightly different info when following the steps and using an invalid date. Also, the patch provided for admin-functions.php is no longer is a valid patch. I took a look at what that patch was changing and found where in the existing code that the code was located in. The code that was being changed by this patch is now located in wp-admin/includes/post.php. I have created a patch that incorporates the changes from the original patch for admin-functions.php in post.php. I did not incorporate the suggestion made by f00f to change the time normalization. I don't fully grasp modulus and so am leaving that to someone else, unless someone can verify that the suggestion made by f00f is correct and would work. I'll post the patch here for testing.

JDTrower6 years ago

Fix to make sure all posts created/updated have valid post_date fields based on the patch for admin-functions.php.

comment:13 santosj6 years ago

The modulus gives the reminder (if you didn't already know)

So for 0-23, it will give you 0-23. When you have more than that then it will start returning the reminder.

24 = 0 which is right, because it would be the next day.
25 = 1 which is correct since 25-24 = 1.
48 = 0
49 = 1


... so on.

It would be better since if someone enters a number higher than 48, then it will return a value greater than 24, which is incorrect. Doing a while look is inefficient, since the modulus does everything you need. If you wanted to advance the day, then a while loop would be better, but you could just use an divide to get how many days should be advanced.

comment:14 JDTrower6 years ago

Should we be concerned with advancing the day? I am all for simplifying the time normalization using modulus. The way you described it makes complete sense. I just don't think we need to be concerned with advancing the day. I'll go ahead and change the time normalization to use modulus and post the new patch.

comment:15 santosj6 years ago

I wouldn't advance the day. It was an example of a possible use case. I would just assume that the user made a mistake and try to fix it. They can go back and correct the hour/minute/ whatever later.

Yo. Thanks.

comment:16 JDTrower6 years ago

Ok, I am adding a new patch that makes the changes that f00f suggested for time normalization along with the code to ensure that all posts have valid dates.

JDTrower6 years ago

Fix to make sure all posts created/updated have valid post_date fields based on the patch for admin-functions.php and makes changes to time normalization.

comment:17 JDTrower6 years ago

  • Keywords needs-testing added; admin write post early removed

comment:18 lloydbudd6 years ago

  • Milestone changed from 2.5 to 2.6

comment:19 ryan6 years ago

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

see #7230

Note: See TracTickets for help on using tickets.