So here's my analysis of what I did back then. I think it was short and
even straight-forward, anyone is welcome to rewrite or reimplement it.
Enough cleanup has happened in url-http.el that it makes it difficult to
simply give you a diff today. I don't even know where to get the code as it
was in 2012 to compare my patch from back then to.
But there were "two" bugs.
Bug one: in url-http-handle-authentication, there is a call to
url-retrieve-internal, and that function can return a new buffer with the
contents of the url, but url-http-handle-authentication does not return the
new buffer with the new contents. It drops it on the floor.
So all I did, literally, was to capture the return value and then change
url-http-handle-authentication throughout so that the the new url contents
would be passed back.
in url-http-handle-authentication, the end of the function goes from:
(if (not (url-auth-registered type))
(progn
(widen)
(goto-char (point-max))
(insert "
Sorry, but I do not know how to handle " (or type
auth url "")
" authentication. If you'd like to write it,"
" please use M-x report-emacs-bug RET.
")
;; We used to set a `status' var (declared "special") but I can't
;; find the corresponding let-binding, so it's probably an error.
;; FIXME: Maybe it was supposed to set `success', i.e. to return
t?
;; (setq status t)
* nil) ;; Not success yet.*
(let* ((args (url-parse-args (subst-char-in-string ?, ?\; auth)))
(auth (url-get-authentication auth-url
(cdr-safe (assoc "realm" args))
type t args)))
(if (not auth)
t ;Success.
(push (cons (if proxy "Proxy-Authorization" "Authorization") auth)
url-http-extra-headers)
(let ((url-request-method url-http-method)
(url-request-data url-http-data)
(url-request-extra-headers url-http-extra-headers))
* (url-retrieve-internal url url-callback-function*
* url-callback-arguments))*
* nil))))) ;; Not success yet.*
to
(if (not (url-auth-registered type))
(progn
(widen)
(goto-char (point-max))
(insert "
Sorry, but I do not know how to handle " type
" authentication. If you'd like to write it,"
" send it to " url-bug-address ".
")
(setq status t)
*(setq retval nil)*)
(let* ((args (url-parse-args (subst-char-in-string ?, ?\; auth)))
(auth (url-get-authentication auth-url
(cdr-safe (assoc "realm" args))
type t args)))
(if (not auth)
(progn
(setq success t)
*(set retval nil)*)
(push (cons (if proxy "Proxy-Authorization" "Authorization") auth)
url-http-extra-headers)
(let ((url-request-method url-http-method)
(url-request-data url-http-data)
(url-request-extra-headers url-http-extra-headers)
* (response-buffer nil)*)
* (setq response-buffer*
* (url-retrieve-internal url url-callback-function*
* url-callback-arguments))*
(url-http-debug "Handling authentication return buffer is %s"
response-buffer)
*(setq retval response-buffer)*))))
(url-http-debug "Handling authentication retval is %s 2:" retval)
* retval))*
At an appropriate place above this, the declaration of retval is stuffed
into an existing let.
The next bug occurs in url-http-parse-headers where the 401 and 407 cases
which deal with authentication do not take into account that the url
contents may have changed as a result of authentication. And there all I
did was to cut and paste the previously self-declared hack from the 3XX
case in.
So the 3XX case had this code already in it
(progn
;; Remember that the request was redirected.
(setf (car url-callback-arguments)
(nconc (list :redirect redirect-uri)
(car url-callback-arguments)))
* ;; Put in the current buffer a forwarding pointer to
the new*
* ;; destination buffer.*
* ;; FIXME: This is a hack to fix
url-retrieve-synchronously*
* ;; without changing the API. Instead url-retrieve
should*
* ;; either simply not return the "destination" buffer,
or it*
* ;; should take an optional `dest-buf' argument.*
* (set (make-local-variable 'url-redirect-buffer)*
* (url-retrieve-internal*
* redirect-uri url-callback-function*
* url-callback-arguments*
* (url-silent url-current-object)))*
* (url-mark-buffer-as-dead buffer))*
I diligently copied that code into the 401 and 407 cases.
(`unauthorized ; 401
;; The request requires user authentication. The response
;; MUST include a WWW-Authenticate header field containing a
;; challenge applicable to the requested resource. The
;; client MAY repeat the request with a suitable
;; Authorization header field.
(url-http-handle-authentication nil))
to
(unauthorized ; 401
;; The request requires user authentication. The response
;; MUST include a WWW-Authenticate header field containing a
;; challenge applicable to the requested resource. The
;; client MAY repeat the request with a suitable
;; Authorization header field.
;; bug patch because url-http-handle-authentication
;; might return a new buffer
(let ((retval (url-http-handle-authentication nil)))
(url-http-debug "Url Http Parse Headers: handling
authentication return buffer TO %s" retval)
(when retval
;; Put in the current buffer a forwarding pointer to the new
;; destination buffer.
;; FIXME: This is a hack to fix url-retrieve-synchronously
;; without changing the API. Instead url-retrieve should
;; either simply not return the "destination" buffer, or it
;; should take an optional `dest-buf' argument.
(set (make-local-variable 'url-redirect-buffer)
retval)
(url-http-debug "Url Http Parse Headers: handling
authentication return buffer TO %s -> %s 2:"
retval url-redirect-buffer)
(url-mark-buffer-as-dead buffer))))
And I did the same thing for the 407 case.
I didn't test it much back in 2012. It worked for my needs against
posterous, which then went belly up. You should probably test this now more
than I did then, when I wasn't writing so much of a patch, as indicating
where the bugs were and the proper fixes needed to apply.
I hope that helps,
Jerry
On Sat, Jun 3, 2017 at 4:05 AM, Eli Zaretskii wrote:
> > From: David Engster
> > Date: Sat, 03 Jun 2017 12:41:47 +0200
> > Cc: Jerry Asher , 10478@debbugs.gnu.org
> >
> > I'm not sure if the resulting patch will fit in the 'trivial patch'
> > category.
>
> It's hard to tell, as no diffs were posted.
>
> Would it be possible to extract the error-handling part into a
> separate function, so that the actual changes would be as short as
> possible? Then we could see if they are short enough to accept
> without papers.
>
> Thanks.
>