Converting & in image name to ?amp;

Filenames shouldn't have ampersands in them. Stop breaking the internet.

But theoretically, if you passed Pathologic a path with an ampersand in it, it should be converted to %26, not &amp (or ?amp). I might have to experiment a bit and look into why that's happening later. Probably won't be soon, though, unfortunately.

Just a note: because the pathologic filter is intended to run after the HTML filter, it always receives an escaped ampersand (&).

Next, please note that http://example.com/foo&bar is valid per RFC 2396 and should therefor not be converted to ?; it is not part of the query string.

3.3. Path Component

   The path component contains data, specific to the authority (or the
   scheme if there is no authority component), identifying the resource
   within the scope of that scheme and authority.

      path          = [ abs_path | opaque_part ]

      path_segments = segment *( "/" segment )
      segment       = *pchar *( ";" param )
      param         = *pchar

      pchar         = unreserved | escaped |
                      ":" | "@" | "&" | "=" | "+" | "$" | ","

   The path may consist of a sequence of path segments separated by a
   single slash "/" character.  Within a path segment, the characters
   "/", ";", "=", and "?" are reserved.  Each path segment may include a
   sequence of parameters, indicated by the semicolon ";" character.
   The parameters are not significant to the parsing of relative
   references.

We appear to have crossposted.

Filenames shouldn't have ampersands in them. Stop breaking the internet.

A small aside: A URI is not necessarily a filename.

Further from the RFC

Unreserved characters can be escaped without changing the semantics
   of the URI, but this should not be done unless the URI is being used
   in a context that does not allow the unescaped character to appear.

Yes, you're correct, of course. But it's easier for me to blame you for using the module in an unanticipated or untested way than for me to admit and correct any fault in the code.

Just a note: because the pathologic filter is intended to run after the HTML filter, it always receives an escaped ampersand (&).

Well, I would hope that the HTML filter would be wise enough to not escape values in HTML attributes. But even if not, I don't think it's the culprit here, since you mentioned that the problem was still happening even after disabling all other filters. And I can't imagine what in Pathologic's code would be converting & to ?. But as I said, I'll experiment once the holidays stop spinning me around.

I do not use the pathologic filter. I also missed your joke :)

I would hope that the HTML filter would be wise enough to not escape values in HTML attributes

The HTML filter is a wrapper for filter_xss and it has to make sure there are no unescaped ampersands in the filtered text. The snippet below is invalid HTML due to the unescaped ampersand:

<a href="http://example.com/index.php?a=foo&b=bar">text</a>

It should be:

<a href="http://example.com/index.php?a=foo&amp;b=bar">text</a>

So, when the HTML filter is disabled, one has to manually enter &, giving pathologic yet again an entity.

The snippet below is invalid HTML due to the unescaped ampersand:

<a href="http://example.com/index.php?a=foo&b=bar">text</a>

Unless I've been wrong about this for over a decade, that HTML is perfectly valid because the ampersand is unambiguously in a value of an HTML tag attribute. But maybe we're confusing the issue…?

(Also, my bad for not noticing that Heine wasn't the OP.)

I'm very sorry, but you've been wrong for a decade. Blame the forgiving useragents!

We're not confusing the issue; there really are two issues here.

1. You need to make sure that your filter correctly deals with & in attributes, not &.
2. Then you need to make sure you do not interpret parts of the URI path as query string (converting & to ?).

If you still want to support invalid HTML, you could opt for supporting a single & in addition to & as well.

lambic, I just did some work on this and believe I may have solved it. I committed my changes; when a new dev release becomes available, please download it and give it a try and see if it corrects this problem for you (note that you'll have to clear your site's cache or edit and change the content that path is in before you'll see an effect).

Grr. Stupid CVS.

I'll do another push when I can, but the code is at home and I'm at work at the moment. I meant to do so last night, but it slipped my mind.

All right, I can't figure out how to get CVS to stop being stupid and there doesn't seem to be anybody on IRC at the moment who can tell me how to get CVS to stop being stupid, so here's a stupid archive.

Just to be clear, by "looks good," you mean it's now working as expected?

I went ahead and made a new full beta release which incorporates these changes. Please test it out if you have a moment and confirm that it still works.

In that case, I'll go ahead and close this issue out. Please feel free to re-open it if you see any similar issues pop up.

I don't know if you'll want anything to do with this, but there's a remaining problem - describing here for people who might run into it.

i'm finding that with clean_urls and Pathologic enabled, links to files that have ampersands in their names no longer work, the ampersands getting changed to '%2526'. it's url() that's doing it, which as far as i can tell is for the reason described in #119876: UNable to use Ampersand within Wiki title - it's double-encoded for the clean URL. links to a Drupal path that contains an ampersand work fine, so it looks like that's working OK. the problem with file links is that they don't get decoded, because they're not known to Drupal. they're just files on the file system.

to get around this, i added code to Pathologic after the url() call that undoes the ampersand encoding if the $paths['path'] value is a file. probably it should handle other characters as well, this is just the only one that was a problem for us.