Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The optional <content:encoded>
element defines the full content of an <item>
. Its value should be either entity-encoded or CDATA-escaped version of the content of the item.
Current format_xml_elements()
implementation though does not allow for CDATA-escaped values (or unencoded in general) - they always get entity-encoded.
Resources:
Proposed resolution
Modify format_xml_elements()
implementation to allow unencoded values if requested directly (by adding new encoded
property to XML element). Keep current behaviour if property is not set to ensure full backwards compatibility.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#30 | drupal-wrong_change_log-2393461-30.patch | 628 bytes | Sagar Ramgade |
#27 | 2393461-wrong-changelog.patch | 631 bytes | nevergone |
#20 | interdiff.txt | 777 bytes | David_Rothstein |
#20 | 2393461-20.patch | 1.42 KB | David_Rothstein |
#17 | drupal-format_xml_elements_unencoded_7.x-2393461-17.patch | 1.38 KB | davic |
Comments
Comment #1
maciej.zgadzaj CreditAttribution: maciej.zgadzaj commentedProposed patch fixing the issue.
Comment #2
mpv CreditAttribution: mpv commented@maciej.zgadzaj thanks, I like your solution. I'm attaching the patch for drupal 7.x in case anyone finds it useful.
Comment #3
wonder95 CreditAttribution: wonder95 commentedI've been using this for 5 months now, and it works perfectly. I'm still re-applying it after every core update for D7, so it would be nice if it could be committed.
Comment #5
Andre-B#2 works for me
Comment #6
dawehnerWe killed that in the meantime.
Comment #7
Andre-Bhow and when was this solved for d7?
Comment #8
longwaveReopening for 7.x, someone should upload #2 as a .patch file for it to be tested.
Comment #9
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer commentedHere's the patch from #2.
Comment #10
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer commentedComment #11
drikc CreditAttribution: drikc commentedSuccessfully skip item value encoding by using the encoded property!
Comment #12
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThe patch looks good, but I think the new option definitely needs to be mentioned in the function documentation. (Especially since it has security implications - we need to be really clear in the documentation when it should/shouldn't be used.)
Comment #13
mpv CreditAttribution: mpv at gcoop Cooperativa de Software Libre commentedHere's the patch with updated funcion documentation.
Comment #14
mpv CreditAttribution: mpv at gcoop Cooperativa de Software Libre commentedComment #15
wonder95 CreditAttribution: wonder95 commentedThis has been out there for a year and a half, and it now has function documentation as well as a test. Can we please get this committed so that I don't have to manually re-add this patch after every update?
Comment #16
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedWe usually don't reference issues in code in D7 - except for @todo's.
Lets also add something like:
"Only use this option if you have already escaped the data as not doing so has security implications."
Comment #17
davic CreditAttribution: davic at CI&T commentedRemoved the issue reference as it's not a common use in drupal 7 (see #16) and added the comment suggestion by Fabianx.
Comment #18
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedRTBC, Stefan will need to commit this.
Comment #19
stefan.r CreditAttribution: stefan.r commentedDavid, are you happy with docs here?
Comment #20
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedMostly looks good, but the new sentence should be part of the previous paragraph - otherwise it's not clear what it refers to.
Also, if you CDATA-escape untrusted input, that doesn't make it safe (at least not when done naively, and I'm not aware of any function in Drupal or PHP that helps you do CDATA-escaping safely).
So thinking about this a bit more... maybe we should be safe and always just recommend entity-encoding in the case where you're working with untrusted user input? (My understanding is that they are basically interchangeable anyway.)
Like the attached, in other words.
Comment #21
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedSaving issue credits - I think there is a bug on drupal.org that sometimes makes them get out of whack.
Comment #22
stefan.r CreditAttribution: stefan.r commentedI think the current wording makes sense...
Comment #23
stefan.r CreditAttribution: stefan.r commentedComment #25
stefan.r CreditAttribution: stefan.r commentedCommitted and pushed to 7.x, thanks!
Comment #26
stefan.r CreditAttribution: stefan.r commentedComment #27
nevergone CreditAttribution: nevergone commentedWrong changelog.
Comment #29
stefan.r CreditAttribution: stefan.r commentedThanks @nevergone
Comment #30
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer commented@stefan just noticed a small typo in the above patch, there are two "a":
Comment #31
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer and at Trigyn Technologies Ltd commentedComment #34
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer and at Trigyn Technologies Ltd commentedComment #36
wonder95 CreditAttribution: wonder95 commentedThanks for getting this done and committed, everyone. This will be a huge headache saver for me.