Problem/Motivation
The API function taxonomy_implode_tags() is passed an array of term objects and returns an imploded string to provide the default value of a taxonomy field. However, the function modifies the object's $term->name
property, rather than creating a copy:
if (strpos($tag->name, ',') !== FALSE || strpos($tag->name, '"') !== FALSE) {
$tag->name = '"' . str_replace('"', '""', $tag->name) . '"';
}
Proposed resolution
Don't modify $term->name
; instead, save the desired value to the list directly. Patch in #3 implements this change.
Remaining tasks
None. Taxonomy maintainer has approved the patch.
User interface changes
None.
API changes
None.
Original report by @BayerMeister
Calling the function taxonomy_implode_tags not only returns imploded tags, but also modifies the array it has as a parameter.
What are the steps required to reproduce the bug?
It suffices to call the function twice with an array containing one tag like "John Smith, PhD." (containing a comma or a double-quote).
The first call modifies the input array so that the tag is additionally surrounded with double quotes.
The second call receives an array containing ""John Smith, PhD."". This is an unwanted and undocumented side-effect which is trivial to fix
What behavior were you expecting?
Just as documented: Should return something, takes something as parameter (implicitly does not modify).
What happened instead?
Modifies input.
I'm attaching a simple patch. This is the same for D6 except line numbers and a few differences in context.
Comment | File | Size | Author |
---|---|---|---|
#16 | taxonomy-1078398-16.patch | 725 bytes | jyotisankar |
#14 | taxonomy-1078398-14.patch | 727 bytes | jyotisankar |
#10 | 1078398-taxo-tags.patch | 725 bytes | andypost |
#3 | 1078398-3.patch | 746 bytes | xjm |
noTagModify.patch | 645 bytes | BayerMeister | |
Comments
Comment #1
xjmTracked down this bug today after beating my head on #1232686: TAC causes term reference autocomplete values containing commas to be triple-quoted. The patch looks like it should resolve the issue without any side effects.
Comment #3
xjmOld patch format. Here's a reroll in the new format.
Comment #4
xjmComment #5
xjmComment #6
sunThanks - don't think this can or should be tested. Typical PHP5 OO drama.
Comment #6.0
xjmAdded summary.
Comment #7
catchLooks good.
Comment #7.0
catchAdded link to patch.
Comment #8
xjmMarked #1232686: TAC causes term reference autocomplete values containing commas to be triple-quoted as duplicate of this issue.
Comment #9
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. Good catch.
Comment #10
andypostRe-roll for 6.x
Comment #11
valthebaldGood in D6 too
Comment #11.0
valthebaldUpdated issue summary.
Comment #13
valthebaldComment #14
jyotisankar CreditAttribution: jyotisankar commentedComment #15
valthebaldSmall formatting fix: please remove extra space on line 1400 (else clause)
Comment #16
jyotisankar CreditAttribution: jyotisankar commentedExtraspace removed.
Comment #17
valthebaldLooks good!