Spaces in URL are not properly encoded in sent email

darrelf,

I tried this. It worked just fine. Are you still having problems?

Still the same problem with Gmail.
If there is a space in the file name (translated to %20) gmail won't show the image.

Using Drupal 4.7.1 with MIME CVS and SEND 4.7.0

This is mimemail's problem (confusing, I know!)

Not seeing a clean fix for this as I believe it's an issue with the base64 encoding and how that's being decoded by specific clients. This would need more testing... If anyone has any additional insight into causes and possible fixes for this, please let us know here.

See also http://drupal.org/node/253859 , which I'm closing as a dupe, but may include some insight.

Well, the correct answer is that URLs aren't allowed to have spaces in them. If Send is letting spaces through, then I think that is the issue. Mimemail has one too though: it will call urlencode() on the %20 and give you %xx20. This is because _mimemail_url() calls url() at some point, which calls urlencode().

The solution, I think, is that url() isn't the right function. Our job isn't to fix up invalid URLs. Additionally, if clean URLs are off, it'll start throwing index.php?q= in there. But it also causes this issue. My patch simply makes the URL absolute. It doesn't do any of the other things inside url(). I think this should be ok, as my current understanding is that the URLs should already be sanitized. But if this is wrong and we need to be going through url(), a simple preg_replace() could fix the + and index.php?= issue.

Anyway, with this patch I can encode spaces, and anything else just fine.

Looking at this further, I think the real solution will be to simply do a str_replace(" ", "%20", $url); on the $url before parsing, and also on the $file for fully qualified image references. Before implementing, I'd like to finish testing and commit a patch for issue http://drupal.org/node/291903.

Hmm. I don't think that will work. The problem is, url() will urlencode() that url, and the %20 will become %xx20. If we're determined to fix up invalid URLs, we could take my patch and add rawurlencode(rawurldecode($url)). url(), I think, is designed for building drupal URLs from drupal paths. If clean URLs are off, it does the wrong thing, and the URL contains valid encoded entities, it does the wrong thing. The only time it does the right thing is when it does basically what my code does.

Actually, urlencode() called within url() replaces spaces with '+' as documented here: http://us2.php.net/urlencode

Other than that, the urlencode() within url() will make sure anything else is cleaned up appropriately, and will not affect spaces replaced with %20 through other means, whether that be str_replace() or rawurlencode(). I considered using rawurlencode() however it seemed that simply replacing the spaces would be the simplest fix.

Additionally, this patch actually does not fix the root issue, which was spaces breaking image references. Image references are handled by the file functions we're working on separately, and would not be touched by the url functions we're dealing with here. So whatever we implement to fix spaces here will need to also be implemented for absolute file references.

I think I didn't explain properly, and I realize now probably should've filed a seperate issue. My problem was slightly different, though the result of the same bug. _mimemail_url() is used to patch up URLs in links to be sure they are absolute. But in the process, it calls urlencode() on the already encoded url. This leads to double encode issues. To reproduce, simply create a valid link with a space encoded as %20. The link in the message will come out as %xx20, because the % sign is not an alphanumeric character and thus gets encoded. Like I mentioned above, url() will also break when clean URLs is off, because it assumes it is given a drupal path, when in our case, it is in fact an already valid URL, it just needs to be absolute.

mathewbot - how are you reproducing this? I've tested this on my end with Send and Mimemail - and am unable to reproduce. Even with URLs that already contain %20 instead of spaces, it does not get encoded twice or munged by url()

Well, I was posting that from memory because, in actuality I'm currently away from my development machine. Looking at the code behind url() again, it has a seperate path for absolute URLs that I was previous unaware of (Again, I like simple functions with simple inputs and outputs, though I need to learn they don't all exist like that :) ). So, I'm not sure how my problem would be possible. The index.php?q= was entirely speculation, but I thought I found an issue with links. My only theory besides the issue not really existing is that maybe its something to do with Simplenews, which is what I was using to test.

I'll check back when I can and see if I can reproduce my link with space issue, though it seems like it might originate in Simplenews. If I can find it I'll open a seperate issue. Sorry to hold up your fix!

Ok, here's the results of some testing I did. I've got a better understanding of the problem, and I do think it falls under this issue's title, even if the OP's problem isn't caused by the exact same code that causes my issue. Here's the testing:

Excluding the Clean URLs off test, Clean URLs were on. All tests included an image with a space in it and a link to a document with a space in it. All attachment tests were run with my latest patch from #291903: Properly convert file URLs to paths.

Absolute URLs (http://www.site.com/sitedir/whatever)
Literal space: Space goes through, gmail converts it to a +.
Encoded space: Correct results.
Clean URLs off, Encoded space: Correct results.

Site-relative URLs attachment tests (/sitedir/whatever)
Literal space: Image properly attaches. For the link, space goes through, gmail converts it to a +.
Encoded space: Image properly attaches. Link, however, becomes %2520.
Clean URLs off, Encoded space: Image properly attaches. Link, however, includes index.php?q= and %2520.

So, it looks like my issue (bold) only crops up with links (looking back, images don't even pass through _mimemail_url()), and only when the URL is site-relative. I didn't realize it didn't apply to absolute URLs, but could this be why you were unable to reproduce it jerdavis? The issue is because url() thinks the site relative URL is a drupal path when it is not.

The OP's issue (italic) appears to be a gmail quirk. The raw output simply includes the space verbatim, gmail decides to make it into a space.

Now, applying my fixed no_urldecode2 while keeping the other patch:

Absolute URLs
Literal space: Correct Results.
Encoded space: Correct results.
Clean URLs off, Encoded space: Correct results.

Site-relative URL attachment test:
Literal space: Correct Results.
Encoded space: Correct results.
Clean URLs off, Encoded Space: Correct results.

The fixed no_urldecode patch fixes an off by one error in one of my substr()s, and includes the str_replace() suggested by jerdavis to fix the OP's original issue. In my testing, it (with my newest url to path patch) fixes every combination of URL, space encoding, and clean URL for both images and URLs.

Addressing this in http://drupal.org/node/291903

Fixed in commit for http://drupal.org/node/291903