#3474 closed task (blessed) (fixed)
Introduce `serve-happy` API endpoint
Reported by: | flixos90 | Owned by: | dd32 |
---|---|---|---|
Milestone: | Priority: | normal | |
Component: | API | Keywords: | has-patch has-unit-tests |
Cc: |
Description
Per discussion in [this week's PHP meeting](https://wordpress.slack.com/archives/C60K3MP2Q/p1519056088000267), we would like to implement a basic endpoint for PHP version information handling, particularly to be used by core to check whether to show the PHP upgrade nag (see https://core.trac.wordpress.org/ticket/41191).
The endpoint should live at api.wordpress.org/core/serve-happy/
and do the following:
- Return true/false for whether the passed PHP version needs to be upgraded.
- Return true/false for whether the passed PHP version is insecure (no longer receives security updates).
- Display relevant PHP versions on which the above "decisions" are based, plus WordPress' recommended PHP version.
- Maybe also the latest PHP version available?
- Maybe also the URL to the Servehappy education page? This would allow us to tweak it more flexibly and independently from core updates, just like the version numbers.
Attachments (2)
Change History (16)
This ticket was mentioned in Slack in #core-php by flixos90. View the logs.
7 years ago
#3
follow-up:
↓ 4
@
7 years ago
- Owner set to dd32
- Status changed from new to accepted
I'll take a deeper look into this tomorrow, however, as a few initial notes:
- Returning
latest_php
probably isn't going to be useful to the implementation, nor actually be updated as often as PHP releases are made, so it's probably worth dropping that field entirely. BrowseHappy the website IIRC pulls the data from another source, which I don't think we'd want to implement in an API. - Can we cache the API endpoint responses based on the
REQUEST_URI
alone? (If possible, reliance upon$_POST
and$_SERVER
vars means caching isn't really viable) At present it appears as such. - How is it planned for the
upgrade_url
parameter passed to be host-specific? What data would be used? - Including the URL to
https://wordpress.org/support/upgrade-php/
seems like a worthwhile idea at first, although having it hard-coded in core would allow for locales to translate it into their own languages - which is far more important than it being included in the API.
To clarify:
Return true/false for whether the passed PHP version needs to be upgraded.
The 'upgrade'
flag isn't whether it needs upgrading, but if WordPress.org believes it's out-of-date
. Calling it upgrade
seems like a misnomer, It's not "Oh you're running 7.0.1, so you need to upgrade to 7.0.27".
Two options I thought of while looking at it:
- Combine to single field,
status
=>out_of_date, no_security_updates, ok, latest
(ok
is a bit of a random 'status' here,latest
could be combined into it too) - Rename to more verbose flags such as (for PHP 5.2)
out_of_date
=>true
&receiving_security_updates
=>false
I think we're okay to assume that PHP will retain it's supported versions security deprecation schedule for 7.x, but I question if it'll stay that way come 8.0
, having a insecure_php <= 5.6
might not work too good in a far distant future, one we can cross when we get there (and lets ignore that Dec 3rd -> 31st window where 5.6 gets updates, but 7.0 doesn't :) )
#4
in reply to:
↑ 3
@
7 years ago
Replying to dd32:
Returning
latest_php
probably isn't going to be useful to the implementation, nor actually be updated as often as PHP releases are made, so it's probably worth dropping that field entirely. BrowseHappy the website IIRC pulls the data from another source, which I don't think we'd want to implement in an API.
Agreed, I can see the maintenance cost for that being too high for now, especially since it's not absolutely necessary.
Can we cache the API endpoint responses based on the
REQUEST_URI
alone? (If possible, reliance upon$_POST
and$_SERVER
vars means caching isn't really viable) At present it appears as such.
Right now, the code uses $_REQUEST
to get the parameters passed (php_version
). WordPress typically has used POST requests for all the APIs afaik, but we could also use GET requests for this one instead, like api.wordpress.org/core/serve-happy/1.0/?php_version=5.3
. Should we change the code so that it checks $_GET
instead of $_REQUEST
? Regarding $_SERVER
, that is only used to detect the protocol.
How is it planned for the
upgrade_url
parameter passed to be host-specific? What data would be used?
This is currently unknown. Maybe the IP address could be used. I thought it may be future-proof to include this field and also support it in core. This would mean whenever we get there, we don't need to wait for a core update. But since none of us has concrete ideas yet, we may also get rid of it.
Including the URL to
https://wordpress.org/support/upgrade-php/
seems like a worthwhile idea at first, although having it hard-coded in core would allow for locales to translate it into their own languages - which is far more important than it being included in the API.
I thought about this too. I think we could support a locale
parameter being passed (in the same way as the php_version
parameter). Then this could return the correct URL. There's benefits and downsides to the API approach: The advantage would be that we can simply change the URL if it changes (which it may in the future, per recent discussions). The disadvantage is that it's probably more time-consuming to change URLs for locales in the codebase than in a translation string.
The
'upgrade'
flag isn't whether it needs upgrading, but if WordPress.org believes it'sout-of-date
. Calling itupgrade
seems like a misnomer, It's not "Oh you're running 7.0.1, so you need to upgrade to 7.0.27".
Two options I thought of while looking at it:
- Combine to single field,
status
=>out_of_date, no_security_updates, ok, latest
(ok
is a bit of a random 'status' here,latest
could be combined into it too)- Rename to more verbose flags such as (for PHP 5.2)
out_of_date
=>true
&receiving_security_updates
=>false
That makes sense. I prefer the first approach, with a single status
field. I think we don't need latest
, particularly as it would require us to keep up with the latest PHP version available (see above). Maybe there could be another status like up_to_date
for when the version is >= WordPress' recommended PHP version (that name is probably not good though). That would leave us with 4 statuses. But I don't have a strong opinion on this, we could also just have three (only ok
as "good" status).
I think we're okay to assume that PHP will retain it's supported versions security deprecation schedule for 7.x, but I question if it'll stay that way come
8.0
, having ainsecure_php <= 5.6
might not work too good in a far distant future, one we can cross when we get there (and lets ignore that Dec 3rd -> 31st window where 5.6 gets updates, but 7.0 doesn't :) )
Good point. I'm not sure whether we should change behavior for that, but we could consider not including these version numbers in the API responses. It would however be less transparent to the client then.
#7
@
7 years ago
@flixos90 I left this open incase the initial API had any issues or missing stuff :)
If you can confirm it looks good, fine to close it.
I've left the "out_of_date", "receiving_security_updates"
fields in there along side the status
field for debugging purposes - If you can confirm if you'd like to keep those or remove them or whatever that'd be good.
#8
@
7 years ago
After writing code for https://core.trac.wordpress.org/attachment/ticket/41191/41191.4.diff, I think it's actually better to continue relying on the individual parameters, instead of status
, as for our specific case at this point in core, we need to look at both things individually (out of date and insecure). So the status
property could as well be removed.
The other minor thing that could be improved is: I suggest changing receiving_security_updates
to something like insecure
so that it aligns with the "negative" connotation of out_of_date
, for better consistency. That means if any of the two values is true
, you have something to improve. :)
This ticket was mentioned in Slack in #core-php by flixos90. View the logs.
7 years ago
#10
@
7 years ago
Per today's PHP meeting, we decided to go with the following approach for the API endpoint:
- Only return the WordPress recommended PHP version as an actual number, in a
recommended_version
field. - Return the following boolean flags (based on the passed PHP version):
is_supported
: Whether the PHP version is actively supported. Currently that means anything >= 7.0.is_secure
: Whether the PHP version receives security updates. Currently that means anything >= 5.6.is_acceptable
: Whether the PHP version is still acceptable by WordPress (meaning no notice needs to show in core). Currently that means anything >= 5.3.
- The whole IP address-related functionality should be removed. It's unknown whether it will ever be needed and may also have privacy implications not worth thinking about at this point. Handling host-specific update URLs should rather be possible via core functionality.
To summarize, every API response should only consist of the four fields explained above.
#11
@
7 years ago
3474.2.diff implements the changes requested above, including modified tests to account for this.
With that approach in place, the API v1 can be considered final.
3474.diff introduces the
serve-happy
API endpoint. Details:index.php
file procedurally executes the code, but all implementation details are part of separate classes.config.php
file (it holds the PHP version numbers, the Servehappy page URL etc.).Serve_Happy\API
. The other classes are simple utility classes representing request and response.The main discussion points should probably be: Do we need all the configuration/response data that is returned by this implementation? Do we need more? Keep in mind that some of this may be useful in the future (such as the
upgrade_url
, which could lead to a host-specific upgrade URL), and we may still wanna include it already to have a predefined pattern that we could implement in core already today (in core we could check if that upgrade URL is not empty, and if so, display a link to it).