Opened 8 years ago
Last modified 21 months ago
#24248 new defect (bug)
'guid' not properly escaped
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.5 |
Component: | Posts, Post Types | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description (last modified by )
Probably related issues: #18274 #19248
'guid' being saved in database not properly escaped, example:
http://www.wordpress.dev/?post_type=changeset&p=57
, see the ampersand encode &
It supposed to be &
or at least &
Once 'auto-draft' saved, 'guid' is correct: http://www.wordpress.dev/?post_type=changeset&p=57
Once post is saved as 'draft' or published (triggered 'update post' on auto-draft), 'guid' gets malformed.
Source of issue: inappropriate usage of get_post_field()
function in the wp_insert_post()
get_post_field()
defaults to 'display' context, we not specify context while obtaining field, and in the wp_insert_post()
we are not going to display it anywhere, just get, check, and save again, correct?
Attached patch adds the 'raw' context to usage of get_post_field()
with 'guid'
Attachments (7)
Change History (24)
#3
@
8 years ago
- Keywords needs-unit-tests 3.7-early added; needs-testing removed
- Milestone changed from Awaiting Review to Future Release
- Version changed from trunk to 2.5
Introduced in [6593]. Triggered by the introduction of custom post types in 2.9, since their permalinks can contain an ampersand.
guid-context.patch seems good. Other get_post_field()
instances in wp_insert_post()
should probably use 'raw'
context too.
#6
@
7 years ago
- Milestone changed from 3.7 to 3.8
This is old and there is a concern there are other potential mines in wp_insert_post() as well.
I'm also concerned about tests for this.
#9
@
7 years ago
Tested patch, works well. Can we get this committed? This a huge pain when dealing with syndicated posts.
#10
@
6 years ago
I've attempted to work on your patch after WordCamp SF, but unfortunately, this patch doesn't appear to fix the problem. I've re-rolled your patch to apply to current trunk, and added a unit test, but even with the patch the unit test fails.
The reason for this is that the conversion of "&" to "&" is happening on the database insert, not on the retrieval from the database.
#11
@
6 years ago
The reason for this is that the conversion of "&" to "&" is happening on the database insert, not on the retrieval from the database.
Conversion happening on post sanitization at beginning of this function $postarr = sanitize_post($postarr, 'db');
The 'url fields' are filtered with wp_strip_all_tags()
,
esc_url_raw()
, and
wp_filter_kses()
- the last one converts
&
to
&
Filters added here: https://github.com/WordPress/WordPress/blob/master/wp-includes/default-filters.php#L59-L65
#14
@
5 years ago
24248.diff is a refresh that moves the code changes to post-functions.php
#15
@
5 years ago
- Keywords has-unit-tests added; dev-feedback removed
Refreshed the patch to move the code back to post.php.
#17
@
5 years ago
24248.3.diff is a refresh of 24248.2.diff which just moves the unit-tests lower since they could not apply.
adds 'raw' context to get_post_field() call for 'guid' field