Subject: Re: bug#7017: url-retrieve seems busted

From: Chong Yidong <[email-masked]>
Date: Tue, 08 May 2012 12:52:01 +0800

Seth Mason <[email-masked]> writes:

> If you put the following in a buffer and eval it, you'll get a 404:
>
>     ;; http://httpbin.org/get?x=1
>     ;; eval this buffer
>     (url-retrieve (buffer-substring-no-properties 4 30) (lambda (&rest
> args) (switch-to-buffer (current-buffer))))
>
> If you curl/wget the same URL, it'll work fine.
>
> If you look at the request, it's going to "/get%3fx%3d1". It seems to me
> that the URL is getting improperly encoded for multibyte strings.

Thanks for pointing this out.

Applying url-hexify-string on the entire URL, as the previous patch did,
is wrong.  We musn't hexify reserved characters that are being used in
their special role.  Unfortunately, figuring out when those characters
are being used in their special role requires an implementation of
RFC2396, which I don't think we currently have in Emacs.

Or, the following not-strictly-correct hack leaves out reserved
characters from hexification.

=== modified file 'lisp/url/url.el'
*** lisp/url/url.el	2012-04-26 12:43:28 +0000
--- lisp/url/url.el	2012-05-08 04:46:45 +0000
***************
*** 180,188 ****
    (url-gc-dead-buffers)
    (if (stringp url)
         (set-text-properties 0 (length url) nil url))
    (when (multibyte-string-p url)
!     (let ((url-unreserved-chars (append '(?: ?/) url-unreserved-chars)))
        (setq url (url-hexify-string url))))
    (if (not (vectorp url))
        (setq url (url-generic-parse-url url)))
    (if (not (functionp callback))
--- 180,193 ----
    (url-gc-dead-buffers)
    (if (stringp url)
         (set-text-properties 0 (length url) nil url))
+ 
    (when (multibyte-string-p url)
!     (let* ((reserved-chars '(?! ?# ?$ ?& ?' ?( ?) ?* ?+ ?, ?/ ?: ?\;
! 			     ?= ?? ?@ ?[ ?]))
! 	   (url-unreserved-chars (append reserved-chars
! 					 url-unreserved-chars)))
        (setq url (url-hexify-string url))))
+ 
    (if (not (vectorp url))
        (setq url (url-generic-parse-url url)))
    (if (not (functionp callback))