This is similar to (but not identical to) #399534: Add fragment and query to render_as_link and #470904: Ampersand [&] gets replaced by [&] in the created URLs.

Here is a small patch that addresses the & (and related) issues as well.

AFAIK, the problem is that render() passes check_plain($value) in all cases. But this is then passes through l() -- which runs check_plain() again. So if you write a link from a textfield like "My test that won't work & stuff", then the & and the ' get double-encoded.

CommentFileSizeAuthor
#6 482162-path.patch683 bytesagentrickard
#4 482162-path.patch649 bytesagentrickard
link.patch577 bytesagentrickard

Comments

Crell’s picture

Crell commented

Won't this still run check_plain() either way, since it's after the if() statement?

merlinofchaos’s picture

merlinofchaos commented

Since most fields override render() I don't think this is the right solution.

I think we need to fix this in the make link portion, where we run the link through filter_xss_admin and set 'html' => true on the l() call.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

@Crell

No. You're missing the return statement inside the IF.

@merlin

I'll see what I can do, but my first tests didn't seem to help there.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Title: Text links get double URL encoded. » Link paths run through check_plain() break argument handling
StatusFileSize
new 482162-path.patch649 bytes

Better title. Better patch.

We decode entities before we strip_tags. This allows strings like "Foo & Bar" to be passed to l() without having been run through check_plain() alterations.

kenorb’s picture

kenorb commented

Similar issue: #483036: Slash replaced in Link path by %252F, but it's probably something else.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Version: 6.x-2.5 » 6.x-2.6
StatusFileSize
new 482162-path.patch683 bytes

That other issue is unrelated.

Updated patch for 6.x.2.6.

deviantintegral’s picture

deviantintegral commented
Status: Needs review » Needs work

Does this mean that the check_plain calls in views_handler_field_node->render and render_link can be safely removed? The patch in #6 doesn't fix the original issue of double-encoding the title attribute for me. If that's the solution, I can go through all of the render methods as needed.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs work » Fixed

#7: No, that's something else, and cannot be safely removed. This only affects link paths which are Special.

BTW this one fell through the cracks due to being marked 'needs work'. =)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

kesmeby’s picture

kesmeby commented
Version: 6.x-2.6 » 6.x-2.x-dev

I'm running the latest Dev version of Views (6.x-2.x-dev) and while the above patch is present in this build I'm still running into the problem of it stripping out ampersands from my URLs when I'm using "Output this field as a link"

I'm trying to link to another view and pass a variable. Does views block this as a security risk? While it strips out "&" and "=" it allows #, $, % and * right through.

I apologize if this is the wrong thread to ask this question.

kesmeby’s picture

kesmeby commented
Status: Closed (fixed) » Active

I forgot to reset the status of the issue when I posted earlier.

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Closed (fixed)

The stripping out of ampersands is not due to the check_plain stuff from above, but because Drupal's url() function expends the query string to be provide separately. That would be a completely separate issue.

Ogredude’s picture

Ogredude commented

OK, so what's the solution to make the url() function quit escaping & and = ?

asb’s picture

asb commented

sub

lucascaro’s picture

lucascaro commented

subscribing...