WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#25089 closed defect (bug) (invalid)

XML-RPC server wrongly conflates post_status "future" with "publish"

Reported by: kailasa Owned by: markoheijnen
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: XML-RPC Keywords: 2nd-opinion dev-feedback
Focuses: Cc:
PR Number:

Description

Word Press Core:

/wp-includes/class-wp-xmlrpc-server.php

is broadcasting "future" as "publish" with this code

// Consider future posts as published

	if ( $post_fields['post_status'] === 'future' )
		$post_fields['post_status'] = 'publish';

this presents a problem for someone who really needs to use XMLRPC to talk to the wp database (via wp.getPosts/WP_QUERY et al....) and distinguish between published posts and scheduled posts.

In a real media world (of books and magazines, articles, PHD dissertations etc.) "Published" means "Now available to the public."

I can't really understand why this decision was made and would consider it a bad one. I suppose the author that file is thinking "OK the post is done and ready to go... it's just waiting for the clock to roll around... so it is effectively published." But this just shows ignorance or lack of experience in the real world of media distribution.

If a variable in a framework has 8 discreet assigned values by default, they are there, separate and distinct, for very good reasons. Future is *not* publish. The former does not appear on the blog, the latter does... they are different.

The core code should not be making decisions to override these distinctions and force developers to "unconflate" two values that someone had the not-so-bright idea to make equal.

wp.getPosts has a filter struct as one optional paramater... I should be free to pass "post_type:POST; post_status:future" as a filter so that my wp.getPosts requests only return posts with a time of now/today or earlier (i.e. post_status not equal to "future")

We have two options. We can either get all posts and then in our LiveCode web app framework (revIgniter) write a routine to parse them all for some date range to ignore all that are in the future. This a horrible hack...and will eat CPU...

OR the other option is we hack the core and comment out the offending two lines in

/wp-includes/class-wp-xmlrpc-server.php

(which I have done for now)

But of course, we then break the Golden Rule "Don't Hack The Core!" and must remember to keep copies of our version of class-wp-xmlrpc-server.php to replace the one that comes in on upgrade... but of course this is really bad, because perhaps the next upgrade to WP has major changes to the class-wp-xmlrpc-server.php...We all know this is not best practices....

So I'm forced to go in after each upgrade and comment out the silly decision to tell the world that "consider bananas (future) equal to apples (publish)"

Please remove that code from

/wp-includes/class-wp-xmlrpc-server.php

this opens the door to a feature request to make the XML-RPC framework customizable and extensible. I will submit that as a separate ticket but refer back to this issue. (if some developer would like "future" to be equated with "publish"... ok, fine, but that should be a "theme" option.

Change History (21)

#1 @dd32
6 years ago

Introduced with [19848] for #18433

Although I can't speak for the reasons, From what I can tell, It was probably done as WordPress considers scheduled posts as "published on a future date", and/or XmlRPC clients not having a notion of a scheduled post (it being, a future-dated published post). but I'm only guessing here.

#2 @markoheijnen
6 years ago

  • Milestone changed from Awaiting Review to 3.7
  • Severity changed from major to normal
  • Version set to 3.4

This is just a copy/paste from the other methods but yes this wasn't a smart move. The code need to change a bit but we still will force things to publish/future depending on the date.

I do have to say that you didn't need to hack core since there is a filter to add new methods to the XML-RPC. That also mean overwrite a WP method.

#3 @markoheijnen
6 years ago

#25090 was marked as a duplicate.

#4 @nacin
6 years ago

  • Keywords 2nd-opinion added

If a post has a time in the future, wp_insert_post() automatically converts 'publish' to 'future' and schedules it. So I'm not actually sure if this affects XML-RPC interaction at all, and is just designed to smooth things out for the underlying API and for clients.

#5 @markoheijnen
6 years ago

I do think that too. Even if it works fine we need to check if we can remove it. Since how the code is right now is just confusing.

#6 @kailasa
6 years ago

" I do have to say that you didn't need to hack core since there is a filter to add new methods to the XML-RPC. That also mean overwrite a WP method."

Can you explain to me how to implement this? such that the implementation falls into /wp-content/themes/myTheme

and will not be over written on upgrade? My understanding is that

our revIgniter code is using this call to WP:

return callXMLRPC("wp.getPosts", pBlogID, pUser, pPassword)

we need to do this now:

return callXMLRPC("wp.getPosts", pBlogID, pUser, pPassword, [add filter 'no-future' struct here])

The problem is: the xmlrpc server is issue "publish" as the status for all future posts.

So, if we *can* filter the future posts *before* the serve output routine, that would be great.

How? I am not a word press expert so we need your guidance.

Then we don't even had to add the struct filter to our wp.getPosts call.

#7 @kailasa
6 years ago

and BTW the behavior that automatically turns "future" into "publish" is not the issue. Of course we want that scheduling option just like it is now... changes to xml-rpc will probably not affect this at all.

#8 @markoheijnen
6 years ago

So what is the issue then? You can't have a future post in the past and you can't have a publish post in the future.

#9 @kailasa
6 years ago

  • Keywords reporter-feedback added

I'm sorry for confusing the issue. The "proper behavior" that I am referring to is not the xml-rpc server, but WordPress's method of monitoring future posts and automatically changing the post_status to "publish" when the scheduled date/time arrives. Of course, when that time arrives, it is no longer "future" it is "now"... the problem with the xml-rpc making "future=post" remains.

#10 @SergeyBiryukov
6 years ago

  • Keywords dev-feedback added; reporter-feedback removed

#11 @markoheijnen
6 years ago

  • Owner set to markoheijnen
  • Status changed from new to assigned

#12 follow-up: @nacin
6 years ago

  • Milestone changed from 3.7 to Awaiting Review

#13 in reply to: ↑ 12 @kailasa
5 years ago

Replying to nacin:

I'm curious why this was pushed back to "Awaiting Review"

Again, the issue is simple: the xmlrpc server

/wp-includes/class-wp-xmlrpc-server.php

is wiping out the post status value of "future"

// Consider future posts as published

	if ( $post_fields['post_status'] === 'future' )
		$post_fields['post_status'] = 'publish';

The problem remains: We have differentiated values in the database for a good reason. If I query wp_post, I should be able to filter out any "future" posts so that only those that are currently seen by blog visitors, will be called.

It's just "wrong" for the core code to force conflate the two. We should be able to depend on the date base to return what is there. What is done here would be similar to force-ably doing this:

if ( $post_fields['last_name'] === 'Smith' )
		$post_fields['last_name'] = 'Jones';

Which means we can never target "Smith" when requesting a name list.

As it is now we have to write "hacks" to filter the results ourselves which we do in LiveCode like this:

function wpGetLastPosts pBlogID, pUser, pPassword
# make a call to wp.getPosts:
	put wpGetPosts(sWordpressBlogID, sWordpressUsername,sWordpressPassword) into tA
	put the keys of tA["methodResponse"]["params"]["param"]["value"]["array"]["data"] into tList
	replace "value[" with empty in tList
	replace "]" with empty in tList
	sort tList
    put 0 into k
# We should not need to do the following parsing and evaluation routine on the data.

repeat for each line x in tList
		put "value["&x&"]" into tPost
		put tA["methodResponse"]["params"]["param"]["value"]["array"]["data"][tPost]["struct"]["member[3]"]["value"]["dateTime.iso8601"] into tTemp
		put char 1 to 4 of tTemp into tYear
		put char 5 to 6 of tTemp into tMonth
		put char 7 to 8 of tTemp into tDate
		put (tYear , tMonth , tDate,0,0,0,0) into tPostDate
		convert tPostDate to long internet date
        -- Comparing the date with the actual date, do not show anything from the future
        put (tYear , tMonth , tDate,0,0,0,0) into tPostDateInSeconds
        convert tPostDateInSeconds to seconds
        put the date && the time into tCurrentDateInSeconds
        convert tCurrentDateInSeconds to seconds
        
        if (tPostDateInSeconds > tCurrentDateInSeconds) then
            next repeat
        end if

it would be better if we could simply depend on the database for what is actually in the database, like this

repeat for each line x in tList
		put "value["&x&"]" into tPost
		put tA["methodResponse"]["params"]["param"]["value"]["array"]["data"][tPost]["struct"]["member[4]"]["value"]["post_status"] into tPostStatus
	        
        if tPostStatus = "future" then
            next repeat
        end if

# thus ignoring all posts that are scheduled

I have other plans which will involve calls to wp.getPosts and I *really* need to be able to depend on "future" is not equal to "published" which obviously it is not in the database itself.

Besides, even the conceptual view that a post that has been scheduled is no longer a draft is therefore "published" breaks a core media content production principle, (I'm talking with 40 years experience in this field, if that carries any weight) Ask any newspaper managing editor or book publisher. Much content is created in advance and then it is given a "publication date" in the future... it is not considered "published" until that date actually arrives. There are huge dependencies on being able to "trust" that this is true. But Word Press's xmlrpc server is breaking this "trust." Something is really, only truly published when it has an audience that can see it; advertisements which are getting views; available for click throughs etc.

#14 follow-up: @markoheijnen
5 years ago

I'm unsure if I really got the issue at the time I commented here and I'm fine to delete the two lines in _prepare_post().
The only thing I want to be sure of is that it would not break any behavior for our mobile apps.

#15 in reply to: ↑ 14 @koke
5 years ago

Replying to markoheijnen:

I'm unsure if I really got the issue at the time I commented here and I'm fine to delete the two lines in _prepare_post().
The only thing I want to be sure of is that it would not break any behavior for our mobile apps.

I'm not sure about Android but I think it would break the UI in iOS.

I don't have much to argue since there isn't a detailed documentation for XML-RPC, but wp.getPostStatusList returns this by default:

array(
  'draft'   => 'Draft',
  'pending' => 'Pending Review',
  'private' => 'Private',
  'publish' => 'Published'
);

So I would assume those are the statuses the API recognizes. With that, the mobile apps translate publish + "future date" into "scheduled post". We don't fetch the status list dynamically, but IIRC we based the supported status list on that.

I don't mind adding "future" to that list of supported statuses, but we'll need to fix the apps first if we want this. Even then, I wonder if that would break other clients.

Replying to kailasa:

...this presents a problem for someone who really needs to use XMLRPC to talk to the wp database (via wp.getPosts/WP_QUERY et al....) and distinguish between published posts and scheduled posts.

You don't use XML-RPC to talk to the database, you use it to talk to WordPress.

#16 @nacin
5 years ago

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

If a post has a time in the future, wp_insert_post() automatically converts 'publish' to 'future' and schedules it. So I'm not actually sure if this affects XML-RPC interaction at all, and is just designed to smooth things out for the underlying API and for clients.

I said this 13 months ago. I just confirmed again this is how it works. XML-RPC converts future to publish, then wp_insert_post() converts it back if the date is in the future. There is no bug here. There is no unexpected behavior here.

I've never even used 'future' as a value for wp_insert_post(). It isn't necessary and it isn't meant to be used directly. It's an internal only status that is overridden anyway. 'publish' should be used.

#17 @kailasa
5 years ago

With all due respect: "I've never even used 'future' as a value for wp_insert_post(). It isn't necessary and it isn't meant to be used directly. It's an internal only status that is overridden anyway. 'publish' should be used." is dismissive of real world media distribution scenarios.

Having been in publishing since 1976, and having had an internet site up and running in 1993 (our magazine was distributed on Compuserve board before the domain/DNS system was invented) ergo, I do have a just a tad bit of experience..

A media item (any media item, and a book, or album, or song, article) is not "published" until it is released to the public, typically this is scheduled at a future date. Until then, something that is scheduled is not in fact published and is meant to be virtually offline/unavailable to the public until publication date. This translates directly into the WP blog posts usage requirements. Simply to declare it as "internal only" isn't useful dismissive this requirement

But... not trying to be argumentative...

The use case is really simple: if I call the "latest posts" with XML-RPC..you will get posts with a future date. Which breaks the intention: you enter a future date, so that they are hidden/unavailable to the public until the future. But the XML-RPC conflates the two database fields and "future" posts suddenly appear as "now" posts.

So: put it another way: what is the solution? How can you filter future posts from the XML-RPC stream of published posts? If there is a way, I will gladly drop my request. But I don't see it.

If you want to use want to use XML-RPC to get "all current posts" and sort in descending order so that the latest post is on top. (a very reasonable use case/requirement) By "latest" I mean published today or in the past... so "published in the future" does not qualify.

Presently we have to do date string checks and manipulations to filter out all posts (greater then [the date(today)]) which is, IMHO, "just wrong." when you already have two fields in the database which clearly address the exact requirement I'm talking about:

future [is not equal to] published... it's that simple. and all API's would ideally respect this distinction.

#18 @rmccue
5 years ago

Just to drop a note: the REST API uses filter[post_status]=future (since it's just WP Query under the hood), but also reports the status field value as publish. A normal query to get all posts only returns actually published posts, regardless of authentication. (The reason we still have the filter as future is because a) it's WP Query's way of referring to it, and b) we don't support date queries.)

kailasa, I think the point nacin was making is that the future post status is just an internal flag to say "the post status is publish, but not until post_date". I'd agree that if you get the most recent posts, it shouldn't include future-published posts, but it may break backwards compatibility to change this on the XML-RPC endpoints now.

#19 follow-up: @kailasa
5 years ago

"I'd agree that if you get the most recent posts, it shouldn't include future-published posts, but it may break backwards compatibility to change this on the XML-RPC endpoints now."

Unfortunate.... means more spaghetti code because we can depend of on the database to deliver what is actually there. We have to dig and filter on date strings [>today] after the data arrives.

It would be good to put this in the documentation: "If you fetch current posts using the XMP-RPC, please be aware that you will get all published posts, including those schedule for future distribution. If you need to limit display to only posts for today into the past you will need to also fetch the post date and then write filters on your own to make sure future posts are not included in the stream."

So do I understand that (seems obvious but I want to be explicit) "the REST API uses filter[post_status]=future (since it's just WP Query under the hood), but also reports the status field value as publish." means that it *does not* fetch future posts?

#20 in reply to: ↑ 19 @rmccue
5 years ago

Replying to kailasa:

It would be good to put this in the documentation: "If you fetch current posts using the XMP-RPC, please be aware that you will get all published posts, including those schedule for future distribution. If you need to limit display to only posts for today into the past you will need to also fetch the post date and then write filters on your own to make sure future posts are not included in the stream."

Agreed on documentation changes; it should probably be noted on the codex.

So do I understand that (seems obvious but I want to be explicit) "the REST API uses filter[post_status]=future (since it's just WP Query under the hood), but also reports the status field value as publish." means that it *does not* fetch future posts?

Correct. If you want future posts, you need to explicitly pass filter[post_status]=future, otherwise you just get the currently-published posts.

#21 @kailasa
5 years ago

Understood on the REST API. looking at the codex documentation, and to make one last "on it's death bed" suggestion in case this point I'm trying to make ever gets revisited: you could add "future" to the struct array output, and I doubt this would break any legacy code. But at least if we the response was:

array(

'draft' => 'Draft',
'pending' => 'Pending Review',
'private' => 'Private',
'publish' => 'Published',
'future' => 'future'

);

The developer could use that....

Note: See TracTickets for help on using tickets.