Improve URL encoding
drumm - September 25, 2007 - 22:33
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | drumm |
| Status: | duplicate |
Description
drupal_urlencode() does two things:
1. Changes %2F to slashes, because there is actually no reason to encode slashes.
2. Works around a mod_rewrite bug where the path, the q argument, is un-encoded before it is passed to PHP.
#2 is not relevant for the query string and should not happen there.
- drupal_url_encode() gets another argument to determine the query string or not context.
- drupal_query_string_encode() uses the new argument.
- url() gets better documentation.
- code in core is corrected to properly use these API functions.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal_urlencode_0.patch | 7.06 KB | Ignored | None | None |
#1
Is this a bugfix for the problem i reported in http://drupal.org/node/178432?
#2
Steven would be a great reviewer on this if available.
#3
Is there any example of a bug caused by this in core or contrib?
#4
Updated to HEAD.
What raised this issue for me was doing an autocomplete URL like
http://example.com/autocomplete/string, wherestringwas a URL with a double slash. The second slash got lost due to apache.#5
#6
#7
seems like a regular old bug to me. downgrading.
#8
Shouldn't we get this in?
#9
Could you apply this also to 5.x?
It works great and solves my issue http://drupal.org/node/221915
#10
Just FYI, the print module has had to create a work around around a double URL encoding that happens when using drupal_query_string_encode() #285496: Problem with drupal_query_string_encode().
João
#11
My vote would be for a simpler fix, leaving drupal_urlencode() as it is, but only calling it when it's actually needed. The documentation for drupal_urlencode() should make it clear that it should not be used for query string encoding.
However drupal_query_string_encode() is currently broken: it should not call drupal_urlencode() but should call rawurlencode() or urlencode() instead. This bug seems to have been introduced here: http://drupal.org/node/74070.
comment.module and update.fetch.inc also need correcting to use drupal_query_string_encode() instead of drupal_urlencode() for their query strings.
Apart from these three places, drupal_urlencode() isn't called from anywhere else in Drupal core.
#12
Since, in fact, the only place that Drupal core calls drupal_urlencode() correctly is within url(), perhaps the function should be removed and its code merged into the url() function?
#13
See a separate issue, and patch, for the double-encoding problem for query strings:
http://drupal.org/node/310139
#14
Shouldn't this be marked as a duplicate then?
I prefer the patch in #310139: drupal_query_string_encode() should not call drupal_urlencode()...
Joao
#15