bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Jerry Asher]

From: Jerry Asher <jerry.asher@gmail.com>
To: bug-gnu-emacs@gnu.org
Subject: 24.0.50; url-http-parse-headers can silently drop the
response when handling BASIC AUTHENTICATION
Date: Tue, 10 Jan 2012 23:59:59 -0700
Message-ID: <87zkduafm8.fsf@gmail.com>
--text follows this line--
I am developing an interface to posterous.com.  Posterous has a restful
api described at posterous.com/api.

They require basic authentication.

When making a GET request again their simplest api, users/me that
returns information about the logged in user, url-retrieve-synchronously
will return the wrong buffer.

This seems to be traceable to url-http-parse-headers handling the "401
Basic: Access denied message by" calling url-http-handle-authentication
which prompts the user for a name and password and calls
url-retrieve-internal to retry the request.

This time, url-retrieve-internal will add in the authentication header,
and so the request will succeed in a 200.  At that point
url-retrieve-will return the buffer the response is in,
url-http-handle-authentication will return that buffer, but
url-http-parse-header will drop that new buffer on the floor, silently.

In a similar case, url-http-parse-header will for a 30X redirect uris,
set a buffer local variable url-redirect-buffer.

It would seem that something similar to this needs to be done for 401
Unauthorized basic authentication.



In GNU Emacs 24.0.50.1 (i386-mingw-nt6.1.7601)
 of 2011-09-19 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 6.1.7601
configured using `configure --with-gcc (4.5) --no-opt'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: ENU
  value of $XMODIFIERS: nil
  locale-coding-system: cp1252
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  eldoc-mode: t
  display-time-mode: t
  desktop-save-mode: t
  projectile-global-mode: t
  yas/global-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  savehist-mode: t
  electric-layout-mode: t
  electric-indent-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  show-paren-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: 1

Recent input:
C-s C-s C-s C-l C-a C-n C-n C-n C-n C-n C-n C-f C-a
C-s C-a C-s u r l - h t t p - h a n d l e - a u C-s
C-s C-r C-r C-r C-a C-a C-s u r l - b a s i c - a C-s
C-a C-x 2 C-x o <escape> x f i n d - g r e p <return>
u r l - b u g - a d C-a C-f C-f C-f C-f C-f C-f C-f
- n a m e SPC * <backspace> " * e l " SPC C-e <return>
C-x p C-x p q C-x p C-x 0 C-x o C-a C-s u r l - t <backspace>
r e t r i C-a <escape> C-f C-e <escape> C-b C-s u r
l - h t t p - h a n d l e - a u t h e t <backspace>
n t i c a t i o n C-a C-x o C-x b * U R <tab> <return>
C-s h a n d l i n e <backspace> g C-s C-a C-x o C-x
2 C-x o <escape> x <up> <return> <up> C-e C-b C-b C-b
C-b C-b C-b <escape> <backspace> <escape> <backspace>
h t t p - h a n d l e <return> <escape> < C-s h r l
- <backspace> <backspace> <backspace> <backspace> u
r l - h a n d l <backspace> <backspace> <backspace>
<backspace> t t p - h a n d l e - a C-s C-s C-r C-r
C-a C-l C-x 1 C-e C-b <down-mouse-1> <mouse-1> C-x
3 <escape> x r p o <backspace> <backspace> e p o r
t - b u g <escape> b e m a c s - <return>

Recent messages:
Mark saved where search started
Mark set
Mark saved where search started
Mark set
Mark saved where search started [3 times]
Grep finished (matches found)
Mark saved where search started [3 times]
Grep finished (matches found)
Mark set
Mark saved where search started

Load-path shadows:
c:/users/jerry/d/Dropbox/emacs/site/site-start hides c:/Program Files
(x86)/emacs/emacs-24.0.50/../site-lisp/site-start
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/ezimage hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/ezimage
c:/users/jerry/d/Dropbox/emacs-prelude/personal/custom hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/custom
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/pulse hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/pulse
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/mode-local hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/mode-local
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/inversion hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/inversion
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/data-debug hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/data-debug
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/cedet hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/cedet
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/cedet-idutils
hides c:/Program Files
(x86)/emacs/emacs-24.0.50/lisp/cedet/cedet-idutils
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/cedet-global
hides c:/Program Files
(x86)/emacs/emacs-24.0.50/lisp/cedet/cedet-global
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/cedet-files hides
c:/Program Files (x86)/emacs/emacs-24.0.50/lisp/cedet/cedet-files
c:/users/jerry/d/Dropbox/emacs/site/cedet-1.0/common/cedet-cscope
hides c:/Program Files
(x86)/emacs/emacs-24.0.50/lisp/cedet/cedet-cscope

Features:
(shadow mail-extr message rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mailabbrev gmm-utils mailheader emacsbug grep dired-aux
url-cache apropos cus-edit cus-start cus-load mail-utils network-stream
starttls debug posterous url-http tls url-auth mail-parse rfc2231
rfc2047 rfc2045 ietf-drums url-gw url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-util url-parse url-vars mailcap
paredit find-func multi-isearch mule-util help-mode view time-stamp
vc-git eldoc dired time desktop prelude-xml prelude-scheme prelude-ruby
feature-mode cucumber-mode scss-mode flymake compile derived haml-mode
js json newcomment cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs markdown-mode noutline outline
css-mode ruby-end ruby-block easy-mmode ruby-mode yari ansi-color
prelude-markdown prelude-emacs-lisp prelude-common-lisp prelude-lisp
prelude-c prelude-global-keybindings prelude-editor projectile yasnippet
edmacro kmacro tramp tramp-compat auth-source eieio assoc gnus-util
mm-util mail-prsvr password-cache shell pcomplete format-spec
tramp-loaddefs windmove recentf tree-widget wid-edit savehist saveplace
uniquify electric autorevert delsel prelude-core byte-opt warnings
bytecomp byte-compile cconv macroexp imenu thingatpt prelude-packages
anything-autoloads auctex-autoloads tex-site info easymenu
clojure-mode-autoloads coffee-mode-autoloads deft-autoloads
gist-autoloads haskell-mode-autoloads icomplete+-autoloads
lacarte-autoloads magit-autoloads markdown-mode-autoloads
paredit-autoloads projectile-autoloads sass-mode-autoloads
haml-mode-autoloads scss-mode-autoloads slime-autoloads
synonyms-autoloads wgrep-autoloads yaml-mode-autoloads yari-autoloads
yasnippet-autoloads package tabulated-list prelude-ui url-coding paren
sort cygwin-mount ange-ftp comint regexp-opt ring executable cl server
advice help-fns advice-preload time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 disp-table ls-lisp w32-win w32-vars
tool-bar dnd fontset image fringe lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer button faces cus-face files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
multi-tty emacs)

bug#10478: Possible fix??

[Jerry Asher]

[-- Attachment #1: Type: text/plain, Size: 20481 bytes --]

The following, ugly, changes in url-http.el seem to work, based on very
very very limited testing....

Basically,

I changed url-http-handle-authentication so that if a change of buffer was
made, it would return the new buffer, otherwise it would return nil.

Then in  url-http-parse-headers, I cut and pasted code for the 3XX redirect
case that sets a flag for url-retrieve-synchronously to do the same thing
for the  401 and 407 cases.

I apologize, instead of sending patches, I am just going to attach the two
new functions.

(defun url-http-handle-authentication (proxy)
  (declare (special status success url-http-method url-http-data
    url-callback-function url-callback-arguments))
  (url-http-debug "Handling %s authentication" (if proxy "proxy" "normal"))
  (let ((auths (or (nreverse
    (mail-fetch-field
     (if proxy "proxy-authenticate" "www-authenticate")
     nil nil t))
  '("basic")))
 (type nil)
(url (url-recreate-url url-current-object))
 (auth-url (url-recreate-url
   (if (and proxy (boundp 'url-http-proxy))
       url-http-proxy
     url-current-object)))
 (url-basic-auth-storage (if proxy
    ;; Cheating, but who cares? :)
    'url-http-proxy-basic-auth-storage
  'url-http-real-basic-auth-storage))
 auth
(strength 0)
        (retval nil))

    ;; find strongest supported auth
    (dolist (this-auth auths)
      (setq this-auth (url-eat-trailing-space
       (url-strip-leading-spaces
 this-auth)))
      (let* ((this-type
      (if (string-match "[ \t]" this-auth)
  (downcase (substring this-auth 0 (match-beginning 0)))
 (downcase this-auth)))
     (registered (url-auth-registered this-type))
     (this-strength (cddr registered)))
(when (and registered (> this-strength strength))
  (setq auth this-auth
type this-type
 strength this-strength))))

    (if (not (url-auth-registered type))
(progn
  (widen)
  (goto-char (point-max))
  (insert "<hr>Sorry, but I do not know how to handle " type
  " authentication.  If you'd like to write it,"
  " send it to " url-bug-address ".<hr>")
  (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))


and

;; GET HERE
(defun url-http-parse-headers ()
 "Parse and handle HTTP specific headers.
Return t if and only if the current buffer is still active and
should be shown to the user."
  ;; The comments after each status code handled are taken from RFC
  ;; 2616 (HTTP/1.1)
  (declare (special url-http-end-of-headers url-http-response-status
    url-http-response-version
    url-http-method url-http-data url-http-process
    url-callback-function url-callback-arguments))

  (url-http-mark-connection-as-free (url-host url-current-object)
    (url-port url-current-object)
    url-http-process)

  (if (or (not (boundp 'url-http-end-of-headers))
  (not url-http-end-of-headers))
      (error "Trying to parse headers in odd buffer: %s" (buffer-name)))
  (goto-char (point-min))
  (url-http-debug "url-http-parse-headers called in (%s)" (buffer-name))
  (url-http-parse-response)
  (mail-narrow-to-head)
  ;;(narrow-to-region (point-min) url-http-end-of-headers)
  (let ((connection (mail-fetch-field "Connection")))
    ;; In HTTP 1.0, keep the connection only if there is a
    ;; "Connection: keep-alive" header.
    ;; In HTTP 1.1 (and greater), keep the connection unless there is a
    ;; "Connection: close" header
    (cond
     ((string= url-http-response-version "1.0")
      (unless (and connection
   (string= (downcase connection) "keep-alive"))
(delete-process url-http-process)))
     (t
      (when (and connection
 (string= (downcase connection) "close"))
 (delete-process url-http-process)))))
  (let ((buffer (current-buffer))
 (class nil)
(success nil)
 ;; other status symbols: jewelry and luxury cars
(status-symbol (cadr (assq url-http-response-status url-http-codes)))
 ;; The filename part of a URL could be in remote file syntax,
;; see Bug#6717 for an example.  We disable file name
 ;; handlers, therefore.
(file-name-handler-alist nil))
    (setq class (/ url-http-response-status 100))
    (url-http-debug "Parsed HTTP headers: class=%d status=%d" class
url-http-response-status)
    (url-http-handle-cookies)

    (case class
      ;; Classes of response codes
      ;;
      ;; 5xx = Server Error
      ;; 4xx = Client Error
      ;; 3xx = Redirection
      ;; 2xx = Successful
      ;; 1xx = Informational
      (1 ; Information messages
       ;; 100 = Continue with request
       ;; 101 = Switching protocols
       ;; 102 = Processing (Added by DAV)
       (url-mark-buffer-as-dead buffer)
       (error "HTTP responses in class 1xx not supported (%d)"
url-http-response-status))
      (2 ; Success
       ;; 200 Ok
       ;; 201 Created
       ;; 202 Accepted
       ;; 203 Non-authoritative information
       ;; 204 No content
       ;; 205 Reset content
       ;; 206 Partial content
       ;; 207 Multi-status (Added by DAV)
       (case status-symbol
 ((no-content reset-content)
  ;; No new data, just stay at the same document
  (url-mark-buffer-as-dead buffer)
  (setq success t))
 (otherwise
  ;; Generic success for all others.  Store in the cache, and
  ;; mark it as successful.
  (widen)
  (if (and url-automatic-caching (equal url-http-method "GET"))
      (url-store-in-cache buffer))
  (setq success t))))
      (3 ; Redirection
       ;; 300 Multiple choices
       ;; 301 Moved permanently
       ;; 302 Found
       ;; 303 See other
       ;; 304 Not modified
       ;; 305 Use proxy
       ;; 307 Temporary redirect
       (let ((redirect-uri (or (mail-fetch-field "Location")
       (mail-fetch-field "URI"))))
 (case status-symbol
   (multiple-choices    ; 300
    ;; Quoth the spec (section 10.3.1)
    ;; -------------------------------
    ;; The requested resource corresponds to any one of a set of
    ;; representations, each with its own specific location and
    ;; agent-driven negotiation information is being provided so
    ;; that the user can select a preferred representation and
    ;; redirect its request to that location.
    ;; [...]
    ;; If the server has a preferred choice of representation, it
    ;; SHOULD include the specific URI for that representation in
    ;; the Location field; user agents MAY use the Location field
    ;; value for automatic redirection.
    ;; -------------------------------
    ;; We do not support agent-driven negotiation, so we just
    ;; redirect to the preferred URI if one is provided.
    nil)
   ((moved-permanently found temporary-redirect) ; 301 302 307
    ;; If the 301|302 status code is received in response to a
    ;; request other than GET or HEAD, the user agent MUST NOT
    ;; automatically redirect the request unless it can be
    ;; confirmed by the user, since this might change the
    ;; conditions under which the request was issued.
    (unless (member url-http-method '("HEAD" "GET"))
      (setq redirect-uri nil)))
   (see-other ; 303
    ;; The response to the request can be found under a different
    ;; URI and SHOULD be retrieved using a GET method on that
    ;; resource.
    (setq url-http-method "GET"
  url-http-data nil))
   (not-modified ; 304
    ;; The 304 response MUST NOT contain a message-body.
    (url-http-debug "Extracting document from cache... (%s)"
    (url-cache-create-filename (url-view-url t)))
    (url-cache-extract (url-cache-create-filename (url-view-url t)))
    (setq redirect-uri nil
  success t))
   (use-proxy ; 305
    ;; The requested resource MUST be accessed through the
    ;; proxy given by the Location field.  The Location field
    ;; gives the URI of the proxy.  The recipient is expected
    ;; to repeat this single request via the proxy.  305
    ;; responses MUST only be generated by origin servers.
    (error "Redirection thru a proxy server not supported: %s"
   redirect-uri))
   (otherwise
    ;; Treat everything like '300'
    nil))
 (when redirect-uri
   ;; Clean off any whitespace and/or <...> cruft.
   (if (string-match "\\([^ \t]+\\)[ \t]" redirect-uri)
       (setq redirect-uri (match-string 1 redirect-uri)))
   (if (string-match "^<\\(.*\\)>$" redirect-uri)
       (setq redirect-uri (match-string 1 redirect-uri)))

   ;; Some stupid sites (like sourceforge) send a
   ;; non-fully-qualified URL (ie: /), which royally confuses
   ;; the URL library.
   (if (not (string-match url-nonrelative-link redirect-uri))
               ;; Be careful to use the real target URL, otherwise we may
               ;; compute the redirection relative to the URL of the proxy.
       (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))
     ;; Check existing number of redirects
     (if (or (< url-max-redirections 0)
     (and (> url-max-redirections 0)
  (let ((events (car url-callback-arguments))
 (old-redirects 0))
    (while events
      (if (eq (car events) :redirect)
  (setq old-redirects (1+ old-redirects)))
      (and (setq events (cdr events))
   (setq events (cdr events))))
    (< old-redirects url-max-redirections))))
 ;; url-max-redirections hasn't been reached, so go
 ;; ahead and redirect.
 (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))
       ;; We hit url-max-redirections, so issue an error and
       ;; stop redirecting.
       (url-http-debug "Maximum redirections reached")
       (setf (car url-callback-arguments)
     (nconc (list :error (list 'error 'http-redirect-limit
       redirect-uri))
    (car url-callback-arguments)))
       (setq success t))))))
      (4 ; Client error
       ;; 400 Bad Request
       ;; 401 Unauthorized
       ;; 402 Payment required
       ;; 403 Forbidden
       ;; 404 Not found
       ;; 405 Method not allowed
       ;; 406 Not acceptable
       ;; 407 Proxy authentication required
       ;; 408 Request time-out
       ;; 409 Conflict
       ;; 410 Gone
       ;; 411 Length required
       ;; 412 Precondition failed
       ;; 413 Request entity too large
       ;; 414 Request-URI too large
       ;; 415 Unsupported media type
       ;; 416 Requested range not satisfiable
       ;; 417 Expectation failed
       ;; 422 Unprocessable Entity (Added by DAV)
       ;; 423 Locked
       ;; 424 Failed Dependency
       (case status-symbol
 (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))))

 (payment-required              ; 402
  ;; This code is reserved for future use
  (url-mark-buffer-as-dead buffer)
  (error "Somebody wants you to give them money"))
 (forbidden ; 403
  ;; The server understood the request, but is refusing to
  ;; fulfill it.  Authorization will not help and the request
  ;; SHOULD NOT be repeated.
  (setq success t))
 (not-found ; 404
  ;; Not found
  (setq success t))
 (method-not-allowed ; 405
  ;; The method specified in the Request-Line is not allowed
  ;; for the resource identified by the Request-URI.  The
  ;; response MUST include an Allow header containing a list of
  ;; valid methods for the requested resource.
  (setq success t))
 (not-acceptable ; 406
  ;; The resource identified by the request is only capable of
  ;; generating response entities which have content
  ;; characteristics nota cceptable according to the accept
  ;; headers sent in the request.
  (setq success t))
 (proxy-authentication-required ; 407
  ;; This code is similar to 401 (Unauthorized), but indicates
  ;; that the client must first authenticate itself with the
  ;; proxy.  The proxy MUST return a Proxy-Authenticate header
  ;; field containing a challenge applicable to the proxy for
  ;; the requested resource.

          ;; bug patch because url-http-handle-authentication
          ;; might return a new buffer

          (let ((retval (url-http-handle-authentication t)))

            (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-mark-buffer-as-dead buffer))))

 (request-timeout ; 408
  ;; The client did not produce a request within the time that
  ;; the server was prepared to wait.  The client MAY repeat
  ;; the request without modifications at any later time.
  (setq success t))
 (conflict ; 409
  ;; The request could not be completed due to a conflict with
  ;; the current state of the resource.  This code is only
  ;; allowed in situations where it is expected that the user
  ;; mioght be able to resolve the conflict and resubmit the
  ;; request.  The response body SHOULD include enough
  ;; information for the user to recognize the source of the
  ;; conflict.
  (setq success t))
 (gone                          ; 410
  ;; The requested resource is no longer available at the
  ;; server and no forwarding address is known.
  (setq success t))
 (length-required ; 411
  ;; The server refuses to accept the request without a defined
  ;; Content-Length.  The client MAY repeat the request if it
  ;; adds a valid Content-Length header field containing the
  ;; length of the message-body in the request message.
  ;;
  ;; NOTE - this will never happen because
  ;; `url-http-create-request' automatically calculates the
  ;; content-length.
  (setq success t))
 (precondition-failed ; 412
  ;; The precondition given in one or more of the
  ;; request-header fields evaluated to false when it was
  ;; tested on the server.
  (setq success t))
 ((request-entity-too-large request-uri-too-large) ; 413 414
  ;; The server is refusing to process a request because the
  ;; request entity|URI is larger than the server is willing or
  ;; able to process.
  (setq success t))
 (unsupported-media-type ; 415
  ;; The server is refusing to service the request because the
  ;; entity of the request is in a format not supported by the
  ;; requested resource for the requested method.
  (setq success t))
 (requested-range-not-satisfiable ; 416
  ;; A server SHOULD return a response with this status code if
  ;; a request included a Range request-header field, and none
  ;; of the range-specifier values in this field overlap the
  ;; current extent of the selected resource, and the request
  ;; did not include an If-Range request-header field.
  (setq success t))
 (expectation-failed ; 417
  ;; The expectation given in an Expect request-header field
  ;; could not be met by this server, or, if the server is a
  ;; proxy, the server has unambiguous evidence that the
  ;; request could not be met by the next-hop server.
  (setq success t))
 (otherwise
  ;; The request could not be understood by the server due to
  ;; malformed syntax.  The client SHOULD NOT repeat the
  ;; request without modifications.
  (setq success t)))
       ;; Tell the callback that an error occurred, and what the
       ;; status code was.
       (when success
 (setf (car url-callback-arguments)
       (nconc (list :error (list 'error 'http url-http-response-status))
      (car url-callback-arguments)))))
      (5
       ;; 500 Internal server error
       ;; 501 Not implemented
       ;; 502 Bad gateway
       ;; 503 Service unavailable
       ;; 504 Gateway time-out
       ;; 505 HTTP version not supported
       ;; 507 Insufficient storage
       (setq success t)
       (case url-http-response-status
 (not-implemented ; 501
  ;; The server does not support the functionality required to
  ;; fulfill the request.
  nil)
 (bad-gateway ; 502
  ;; The server, while acting as a gateway or proxy, received
  ;; an invalid response from the upstream server it accessed
  ;; in attempting to fulfill the request.
  nil)
 (service-unavailable ; 503
  ;; The server is currently unable to handle the request due
  ;; to a temporary overloading or maintenance of the server.
  ;; The implication is that this is a temporary condition
  ;; which will be alleviated after some delay.  If known, the
  ;; length of the delay MAY be indicated in a Retry-After
  ;; header.  If no Retry-After is given, the client SHOULD
  ;; handle the response as it would for a 500 response.
  nil)
 (gateway-timeout ; 504
  ;; The server, while acting as a gateway or proxy, did not
  ;; receive a timely response from the upstream server
  ;; specified by the URI (e.g. HTTP, FTP, LDAP) or some other
  ;; auxiliary server (e.g. DNS) it needed to access in
  ;; attempting to complete the request.
  nil)
 (http-version-not-supported ; 505
  ;; The server does not support, or refuses to support, the
  ;; HTTP protocol version that was used in the request
  ;; message.
  nil)
 (insufficient-storage ; 507 (DAV)
  ;; The method could not be performed on the resource
  ;; because the server is unable to store the representation
  ;; needed to successfully complete the request.  This
  ;; condition is considered to be temporary.  If the request
  ;; which received this status code was the result of a user
  ;; action, the request MUST NOT be repeated until it is
  ;; requested by a separate user action.
  nil))
       ;; Tell the callback that an error occurred, and what the
       ;; status code was.
       (when success
 (setf (car url-callback-arguments)
       (nconc (list :error (list 'error 'http url-http-response-status))
      (car url-callback-arguments)))))
      (otherwise
       (error "Unknown class of HTTP response code: %d (%d)"
      class url-http-response-status)))
    (if (not success)
(url-mark-buffer-as-dead buffer))
    (url-http-debug "Finished parsing HTTP headers: %S" success)
    (widen)
    success))

[-- Attachment #2: Type: text/html, Size: 75339 bytes --]

bug#10478: Possible fix??

[Michael Albinus]

Jerry Asher <jerry.asher@gmail.com> writes:

Hi,

> The following, ugly, changes in url-http.el seem to work, based on
> very very very limited testing....
>
> Basically,
>
> I changed url-http-handle-authentication so that if a change of buffer
> was made, it would return the new buffer, otherwise it would return
> nil.
>
> Then in  url-http-parse-headers, I cut and pasted code for the 3XX
> redirect case that sets a flag for url-retrieve-synchronously to do
> the same thing for the  401 and 407 cases.

Maybe there is a simple solution, using `save-current-buffer'?

Best regards, Michael.

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Lars Ingebrigtsen]

Jerry Asher <jerry.asher@gmail.com> writes:

> When making a GET request again their simplest api, users/me that
> returns information about the logged in user, url-retrieve-synchronously
> will return the wrong buffer.
>
> This seems to be traceable to url-http-parse-headers handling the "401
> Basic: Access denied message by" calling url-http-handle-authentication
> which prompts the user for a name and password and calls
> url-retrieve-internal to retry the request.

I can confirm that this bug is still present in Emacs 25.  It returns
the buffer with the 401 message, not the buffer with the response...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[David Engster]

Lars Ingebrigtsen writes:
> Jerry Asher <jerry.asher@gmail.com> writes:
>
>> When making a GET request again their simplest api, users/me that
>> returns information about the logged in user, url-retrieve-synchronously
>> will return the wrong buffer.
>>
>> This seems to be traceable to url-http-parse-headers handling the "401
>> Basic: Access denied message by" calling url-http-handle-authentication
>> which prompts the user for a name and password and calls
>> url-retrieve-internal to retry the request.
>
> I can confirm that this bug is still present in Emacs 25.  It returns
> the buffer with the 401 message, not the buffer with the response...

I just stumpled upon this. Oh my, this stuff is straight from callback
hell. Lars, how's your 'url' rewrite going...?

I stared at this for hours, but was unable to come up with anything
better than what Jerry has suggested. So if nobody objects, I'd like to
commit this.

Jerry, do you have papers with the FSF signed? I'm not sure if the
resulting patch will fit in the 'trivial patch' category.

-David

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Lars Ingebrigtsen]

David Engster <deng@randomsample.de> writes:

> I just stumpled upon this. Oh my, this stuff is straight from callback
> hell. Lars, how's your 'url' rewrite going...?

It's basically usable, but I haven't had time to finish it up, write the
documentation and merge it yet...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Date: Sat, 03 Jun 2017 12:41:47 +0200
> Cc: Jerry Asher <jerry.asher@gmail.com>, 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.

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Jerry Asher]

[-- Attachment #1: Type: text/plain, Size: 8057 bytes --]

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 "<hr>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.<hr>")
          ;; 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 "<hr>Sorry, but I do not know how to handle " type
                  " authentication.  If you'd like to write it,"
                  " send it to " url-bug-address ".<hr>")
          (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 <eliz@gnu.org> wrote:

> > From: David Engster <deng@randomsample.de>
> > Date: Sat, 03 Jun 2017 12:41:47 +0200
> > Cc: Jerry Asher <jerry.asher@gmail.com>, 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.
>

[-- Attachment #2: Type: text/html, Size: 17730 bytes --]

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[David Engster]

Jerry Asher writes:
> 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.

[...]

>          (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))))

That did not work for me. The reason is that at the end of this block,
`url-mark-buffer-as-dead' will return non-nil, so the 'success'-flag
will be set. If the callback is called immediately because
Content-Length is '0' (which is often the case for '401'),
`url-retrieve-synchronously' won't look at the url-redirect-buffer
variable.

So I let that block always return 'nil' and that seems to do the trick,
but I'll do some more testing. In the meantime, I'll send you the form
for assigning copyright to the FSF in a separate mail. [Whether a patch
is considered 'trivial' almost entirely depends on quantity. The general
rule is: If the amount of added lines is below ~10, it is considered
trivial, otherwise papers are required.]

-David

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Lars Ingebrigtsen]

I tried reproducing this bug again now, but I seem to be unable to.  Has
somebody finally fixed it?

Here's my test case:

(pop-to-buffer (url-retrieve-synchronously "https://jigsaw.w3.org/HTTP/Basic/"))

the user name/password is guest/guest.  I'm prompted for that, and the
buffer I'm popped to is the "your browser made it" buffer, so it seems
to work for me?

Does it work for you, too?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Jerry Asher <jerry.asher@gmail.com>,  Eli Zaretskii <eliz@gnu.org>,
>   10478@debbugs.gnu.org
> Date: Tue, 24 Sep 2019 08:55:10 +0200
> 
> (pop-to-buffer (url-retrieve-synchronously "https://jigsaw.w3.org/HTTP/Basic/"))
> 
> the user name/password is guest/guest.  I'm prompted for that, and the
> buffer I'm popped to is the "your browser made it" buffer, so it seems
> to work for me?
> 
> Does it work for you, too?

Seems to work.  I end up in a buffer named " *http jigsaw.w3.org:443*-264606"
(the minus seems weird, perhaps an unrelated misfeature), whose
contents is the entire response from the server, starting with

  HTTP/1.1 200 OK
  date: Tue, 24 Sep 2019 07:11:23 GMT
  content-length: 458
  content-location: https://jigsaw.w3.org/HTTP/Basic/ok.html
  content-type: text/html
  etag: "1lkdfte:qoguo8q8"
  last-modified: Sun, 25 Jun 2000 17:08:58 GMT
  server: Jigsaw/2.3.0-beta3

and ending with this:

  <P>
  <HR>
  <P>Your browser made it!
    </body>
  </html>

Is this what you see?

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[David Engster]

Lars Ingebrigtsen writes:
> I tried reproducing this bug again now, but I seem to be unable to.  Has
> somebody finally fixed it?
>
> Here's my test case:
>
> (pop-to-buffer (url-retrieve-synchronously "https://jigsaw.w3.org/HTTP/Basic/"))
>
> the user name/password is guest/guest.  I'm prompted for that, and the
> buffer I'm popped to is the "your browser made it" buffer, so it seems
> to work for me?
>
> Does it work for you, too?

Yes, works for me.

-David

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Seems to work.  I end up in a buffer named " *http jigsaw.w3.org:443*-264606"
> (the minus seems weird, perhaps an unrelated misfeature),

url-http creates several buffers with `generate-new-buffer' (or the
like), so if there's more than one connection done (as there is here),
you'll get a '-<random>' in the (final) buffer name.

> Is this what you see?

Yup; thanks for testing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10478: 24.0.50; url-http-parse-headers can silently drop the response when handling BASIC AUTHENTICATION

[Lars Ingebrigtsen]

David Engster <deng@randomsample.de> writes:

> Lars Ingebrigtsen writes:
>> I tried reproducing this bug again now, but I seem to be unable to.  Has
>> somebody finally fixed it?
>>
>> Here's my test case:
>>
>> (pop-to-buffer (url-retrieve-synchronously
>> "https://jigsaw.w3.org/HTTP/Basic/"))
>>
>> the user name/password is guest/guest.  I'm prompted for that, and the
>> buffer I'm popped to is the "your browser made it" buffer, so it seems
>> to work for me?
>>
>> Does it work for you, too?
>
> Yes, works for me.

Thanks for testing; I'm closing this bug report as it seems like
somebody must have fixed it in the meantime.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no