url-http.el: redirection to partial URL with a proxy

url-http.el: redirection to partial URL with a proxy

[Klaus Straubinger]

The redirection of a URL retrieval request to a partial (not fully
qualified) URL does not work if the retrieval is attempted with a
proxy connection. The function url-http-parse-headers in url-http.el
constructs a fully qualified URL by using the variable
url-current-object. But this variable contains the proxy URL if a proxy
is used. This leads to a wrong redirection URL.

The following patch solves this problem. It works because
url-http-cookies-sources contains always the original URL as set in the
function url-http. Maybe the variable could be renamed because it is
useful not just for cookie handling.


--- url-http.el.orig	2005-10-21 09:21:56.000000000 +0200
+++ url-http.el	2005-11-14 15:42:07.000000000 +0100
@@ -510,7 +510,9 @@
 	   ;; non-fully-qualified URL (ie: /), which royally confuses
 	   ;; the URL library.
 	   (if (not (string-match url-nonrelative-link redirect-uri))
-	       (setq redirect-uri (url-expand-file-name redirect-uri)))
+	       (setq redirect-uri
+		     (url-expand-file-name redirect-uri
+					   url-http-cookies-sources)))
 	   (let ((url-request-method url-http-method)
 		 (url-request-data url-http-data)
 		 (url-request-extra-headers url-http-extra-headers))


-- 
Klaus Straubinger

Re: url-http.el: redirection to partial URL with a proxy

[Paul Pogonyshev]

Klaus Straubinger wrote:
> The redirection of a URL retrieval request to a partial (not fully
> qualified) URL does not work if the retrieval is attempted with a
> proxy connection. The function url-http-parse-headers in url-http.el
> constructs a fully qualified URL by using the variable
> url-current-object. But this variable contains the proxy URL if a proxy
> is used. This leads to a wrong redirection URL.
> 
> The following patch solves this problem. It works because
> url-http-cookies-sources contains always the original URL as set in the
> function url-http. Maybe the variable could be renamed because it is
> useful not just for cookie handling.

And there's an unneeded `s' at the end, I think I made a typo here :)
Something like `url-http-target-url' may be better.

Paul

Re: url-http.el: redirection to partial URL with a proxy

[Stefan Monnier]

>> The redirection of a URL retrieval request to a partial (not fully
>> qualified) URL does not work if the retrieval is attempted with a
>> proxy connection. The function url-http-parse-headers in url-http.el
>> constructs a fully qualified URL by using the variable
>> url-current-object. But this variable contains the proxy URL if a proxy
>> is used. This leads to a wrong redirection URL.
>> 
>> The following patch solves this problem. It works because
>> url-http-cookies-sources contains always the original URL as set in the
>> function url-http. Maybe the variable could be renamed because it is
>> useful not just for cookie handling.

> And there's an unneeded `s' at the end, I think I made a typo here :)
> Something like `url-http-target-url' may be better.

I'm thinking of installing the change below, but I have two questions:
- the save-excusion;save-restriction;set-buffer thingy looks very fishy.
  My replacingit with with-current-buffer;save-restriction makes it
  look saner, but maybe there was a reason for the fishiness.
- the url-expand-file-name call looks fishy as well because there's
  no reason to assume that url-http-target-url is a directory, so maybe we
  should use
               (setq redirect-uri
                     (url-expand-file-name redirect-uri
                                           (file-name-directory
                                            url-http-target-url)))

I have no understanding of what the code does, so it's all
shallow-debugging.  Can someone confirm what I'm doing looks reasonable?


        Stefan


--- url-http.el	20 oct 2005 15:44:21 -0400	1.22
+++ url-http.el	17 nov 2005 22:43:05 -0500	
@@ -26,10 +26,9 @@
 
 ;;; Code:
 
-(eval-when-compile
-  (require 'cl)
-  (defvar url-http-extra-headers)
-  (defvar url-http-cookies-sources))
+(eval-when-compile (require 'cl))
+(defvar url-http-extra-headers)
+(defvar url-http-target-url)
 (require 'url-gw)
 (require 'url-util)
 (require 'url-parse)
@@ -358,7 +357,7 @@
 work correctly."
   (let ((cookies (mail-fetch-field "Set-Cookie" nil nil t))
 	(cookies2 (mail-fetch-field "Set-Cookie2" nil nil t))
-	(url-current-object url-http-cookies-sources))
+	(url-current-object url-http-target-url))
     (and cookies (url-http-debug "Found %d Set-Cookie headers" (length cookies)))
     (and cookies2 (url-http-debug "Found %d Set-Cookie2 headers" (length cookies2)))
     (while cookies
@@ -510,7 +509,9 @@
 	   ;; non-fully-qualified URL (ie: /), which royally confuses
 	   ;; the URL library.
 	   (if (not (string-match url-nonrelative-link redirect-uri))
-	       (setq redirect-uri (url-expand-file-name redirect-uri)))
+	       (setq redirect-uri
+		     (url-expand-file-name redirect-uri
+					   url-http-target-url)))
 	   (let ((url-request-method url-http-method)
 		 (url-request-data url-http-data)
 		 (url-request-extra-headers url-http-extra-headers))
@@ -727,8 +728,7 @@
   (url-http-debug "url-http-end-of-document-sentinel in buffer (%s)"
 		  (process-buffer proc))
   (url-http-idle-sentinel proc why)
-  (save-excursion
-    (set-buffer (process-buffer proc))
+  (with-current-buffer (process-buffer proc)
     (goto-char (point-min))
     (if (not (looking-at "HTTP/"))
 	;; HTTP/0.9 just gets passed back no matter what
@@ -1039,8 +1039,7 @@
 	  (setq buffer nil)
 	  (error "Could not create connection to %s:%d" (url-host url)
 		 (url-port url)))
-      (save-excursion
-	(set-buffer buffer)
+      (with-current-buffer buffer
 	(mm-disable-multibyte)
 	(setq url-current-object url
 	      mode-line-format "%b [%s]")
@@ -1060,7 +1059,7 @@
 		       url-http-method
 		       url-http-extra-headers
 		       url-http-data
-		       url-http-cookies-sources))
+		       url-http-target-url))
 	  (set (make-local-variable var) nil))
 
 	(setq url-http-method (or url-request-method "GET")
@@ -1073,7 +1072,7 @@
 	      url-callback-function callback
 	      url-callback-arguments cbargs
 	      url-http-after-change-function 'url-http-wait-for-headers-change-function
-	      url-http-cookies-sources (if (boundp 'proxy-object)
+	      url-http-target-url (if (boundp 'proxy-object)
 					   proxy-object
 					 url-current-object))
 
@@ -1096,8 +1095,7 @@
   (declare (special url-http-after-change-function))
   (and (process-buffer proc)
        (/= (length data) 0)
-       (save-excursion
-	 (set-buffer (process-buffer proc))
+       (with-current-buffer (process-buffer proc)
 	 (url-http-debug "Calling after change function `%s' for `%S'" url-http-after-change-function proc)
 	 (funcall url-http-after-change-function
 		  (point-max)
@@ -1114,8 +1112,7 @@
     (defun url-http-symbol-value-in-buffer (symbol buffer
 						   &optional unbound-value)
       "Return the value of SYMBOL in BUFFER, or UNBOUND-VALUE if it is unbound."
-      (save-excursion
-	(set-buffer buffer)
+      (with-current-buffer buffer
 	(if (not (boundp symbol))
 	    unbound-value
 	  (symbol-value symbol))))
@@ -1198,10 +1195,9 @@
     (when (and buffer (= 2 (/ (url-http-symbol-value-in-buffer
 			       'url-http-response-status buffer 0) 100)))
       ;; Only parse the options if we got a 2xx response code!
-      (save-excursion
+      (with-current-buffer buffer
 	(save-restriction
 	  (save-match-data
-	    (set-buffer buffer)
 	    (mail-narrow-to-head)
 
 	    ;; Figure out what methods are supported.

Re: url-http.el: redirection to partial URL with a proxy

[Lennart Borgman]

Stefan Monnier wrote:

>- the url-expand-file-name call looks fishy as well because there's
>  no reason to assume that url-http-target-url is a directory, 
>  
>
Why could it not be a directory?

Re: url-http.el: redirection to partial URL with a proxy

[Stefan Monnier]

>> - the url-expand-file-name call looks fishy as well because there's
>> no reason to assume that url-http-target-url is a directory,  
> Why could it not be a directory?

I'm not sure I understand your question.
Oh, wait, I see that url-expand-file-name, contrary to expand-file-name, does
not assume that the second arg is a directory, so there's no worry.
OK, cool, installed.


        Stefan