Make WordPress Core

Opened 17 years ago

Closed 15 years ago

Last modified 3 weeks ago

#10665 closed defect (bug) (fixed)

Protect XMLRPC against failures when WP_DEBUG enabled

Reported by: redsweater's profile redsweater Owned by: westi's profile westi
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch
Focuses: Cc:

Description

Further protections for xmlrpc.php to guard against errors that are printed when WP_DEBUG is enabled.

The attached patch protects against failures that occur when the optional parameters for enclosures, categories, and ping urls are omitted by the client.

Attachments (2)

XMLRPCSafety.diff (3.1 KB) - added by redsweater 17 years ago.
10665.diff (7.2 KB) - added by sorich87 15 years ago.

Download all attachments as: .zip

Change History (17)

#1 @redsweater
17 years ago

  • Cc jalkut@… added
  • Component changed from General to XML-RPC
  • Owner set to josephscott

#2 @redsweater
17 years ago

I altered the patch to be especially careful that the variables being initialized are set to the same default values they would have received previously. In particular, the problematic array lookups yield initial values of NULL, which I now set explicitly as the fallback value in case the array items don't exist.

I switched from using "!empty" to "isset", so that the logic is identical to before: if the array element is set at all, it is used to initialize the variable. If it's not set, then NULL is used. For categories, an empty array is the default initialized value, which matches previous behavior as well.

#3 @nacin
16 years ago

  • Milestone changed from Unassigned to 3.0

#4 @nacin
16 years ago

  • Milestone changed from 3.0 to 3.1
  • Status changed from new to assigned

#5 @nacin
15 years ago

  • Keywords needs-refresh added; has-patch removed
  • Milestone changed from Awaiting Triage to 3.1

@sorich87
15 years ago

#6 @sorich87
15 years ago

  • Keywords has-patch added; needs-refresh removed

#7 @westi
15 years ago

  • Owner changed from josephscott to westi

I'll look into getting these committed

#8 @automattor
15 years ago

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

(In [16824]) Work around some unset variable notices in xmlrpc.php. props redsweater. props sorich87. fixes #10665

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


7 weeks ago
#9

Part of: https://github.com/WordPress/ai/issues/40
Inspired by the work on https://github.com/galatanovidiu/mcp-adapter-implementation-example/tree/experiment/layerd-mcp-tools/includes/Abilities by @galatanovidiu.

## Summary

This PR adds a core/get-current-user ability to the WordPress Abilities API. This ability returns comprehensive profile details for the current authenticated user,

## Organization

Following the pattern established in #10665 and #10747, this PR adds a new class WP_Users_Abilities in src/wp-includes/abilities/class-wp-users-abilities.php. The class is organized into:

  • Initialization: register() method that registers all user-related abilities
  • Schema Building: get_current_user_input_schema() and get_current_user_output_schema() define the ability's interface
  • Ability Registration: register_get_current_user() registers the ability with schemas and callbacks
  • Execution: execute_get_current_user() retrieves and returns current user data
  • Utilities: get_user_meta_for_response() helper for processing user meta (excluding capability-related keys)

## Key Features

  • Returns the main user fields.
  • Optional include_capabilities parameter to include all user capabilities
  • Optional include_meta parameter to include user meta fields (excludes capability-related meta)
  • Permission check requires user to be logged in
  • Exposed via REST API (show_in_rest: true)

## Test plan

  • Open wp-admin/post-new.php
  • Open the browser console and run the following examples:
// Get current user (basic)
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-current-user/run',
  method: 'POST',
  data: { input: {} }
});
  • [ ] Verify all 15 fields are returned (id, username, email, display_name, first_name, last_name, nickname, description, url, link, slug, registered_date, roles, locale, avatar_urls)
// Get current user with capabilities
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-current-user/run',
  method: 'POST',
  data: { input: { include_capabilities: true } }
});
  • [ ] Verify capabilities object is included in the response
  • [ ] Verify meta is NOT included when only include_capabilities is true
// Get current user with meta
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-current-user/run',
  method: 'POST',
  data: { input: { include_meta: true } }
});
  • [ ] Verify meta object is included in the response
  • [ ] Verify capabilities are NOT included when only include_meta is true
  • [ ] Verify capability-related meta keys (wp_capabilities, wp_user_level) are excluded from meta
// Check the ability schema
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-current-user',
  method: 'GET'
});
  • [ ] Verify the input schema includes include_capabilities and include_meta boolean properties with default false
  • [ ] Verify the output schema documents all fields with types and descriptions

@justlevine commented on PR #10775:


5 weeks ago
#10

(Sorry I missed this one. Wish there was a way we could actually tag other core-ai folks for review. Will take it for a spin hopefully tomorrow or wednesday)

@jorgefilipecosta commented on PR #10775:


4 weeks ago
#11

This PR was rebases and the conflicts were fixed. cc: @justlevine

@justlevine commented on PR #10775:


3 weeks ago
#12

My biggest concern with this PR is that it's not automatically scalable or extendable. It's not obvious (to me or an LLM) why these are the particular set of user data we're returning, we're not exposing user meta that's registered with register_meta( 'user' ...) and a schema, and the permissions model doesn't seem to follow least privilege, nor do I see a clear way to extend it.

@jorgefilipecosta commented on PR #10775:


3 weeks ago
#13

My biggest concern with this PR is that it's not automatically scalable or extendable. It's not obvious (to me or an LLM) why these are the particular set of user data we're returning, we're not exposing user meta that's registered with register_meta( 'user' ...) and a schema, and the permissions model doesn't seem to follow least privilege, nor do I see a clear way to extend it.

Hi @justlevine the plan is to expose user meta. The only reason we are not doing it yet is because I learned the lesson on https://github.com/WordPress/wordpress-develop/pull/10867 😅 and PR's rapidly become very huge and hard to review if we include the full features from the beginning. So the plan is to merge a simple user ability first, and then follow up right away with the meta expansion.

@justlevine commented on PR #10775:


3 weeks ago
#14

PR's rapidly become very huge and hard to review if we include the full features from the beginning. So the plan is to merge a simple user ability first

100% agree. The nuance here is that "simple" means something a bit different for abilities than traditional progressive programming, since it's the schema that needs to scale, and we have a lot more leeway with the internals. My focus isn't why _not_ metadata, but why _yes_ to e.g. locale, link, and esp. capabilities (since it's input _and_ output), and how confident we feel we'll be able to scale it.

Like @aaronjorbin notes in https://core.trac.wordpress.org/ticket/64596#comment:9 , once things are in core _both_ back and forward compat take on a mind on its own, so we need to make sure we can iterate the schema in the holistic direction we want, like how we intentionally prepped for the possibility of nested namespaces

@justlevine commented on PR #10775:


3 weeks ago
#15

This question came up tangentially elsewhere - I couldn't find a trac ticket for this ability , so sharing it here for completeness:

what are we doing with the existing core/get-user-data ability. Functionally, the only difference between the two is whether it's the _current_ user or a specified user, but thats

  • an implementation detail, not an api consideration
  • not self-documented from the name get-user-data versus get-user or even a namespaced user/get (although at least that one implies there's a difference from the fact that its namepaced).
Note: See TracTickets for help on using tickets.