* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
@ 2012-07-18 18:38 David Engster
2012-07-19 6:35 ` Michael Albinus
0 siblings, 1 reply; 11+ messages in thread
From: David Engster @ 2012-07-18 18:38 UTC (permalink / raw)
To: 11981
The function `url-http-parse-headers' temporarily disables all file name
handlers to circumvent a bug where Tramp could kick in because the URL
might look like a remote file (#6717). This is the offending snippet:
;; 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))
However, this workaraound assumes that everything that gets afterwards
in the `let'-section does not depend on file name handlers. However,
this is not the case: `url-http-handle-authentication' is called which
in turn calls the auth-source package. This package however supports
encrypted authinfo files which have to be automatically decrypted
through `auto-encryption-mode'. This, of course, works through file name
handlers.
Here's a backtrace from `auth-source-search' resulting from a call to
`url-retrieve-synchronously' on a password-protected URL:
auth-source-search(:max 1 :host "www.google.com:443" :port "https")
url-do-auth-source-search("www.google.com:443" "https" :user)
url-basic-auth([cl-struct-url "https" nil nil "www.google.com" 443 "/calendar/dav/MY-GOOGLE-ACCOUNT/events/" nil nil t nil t] t nil "Google CalDAV" (("realm" . "Google CalDAV") ("basic")))
url-get-authentication("https://www.google.com/calendar/dav/MY-GOOGLE-ACCOUNT/events/" "Google CalDAV" "basic" t (("realm" . "Google CalDAV") ("basic")))
url-http-handle-authentication(nil)
[ ... ]
url-http-parse-headers()
url-http-chunked-encoding-after-change-function(500 507 7)
url-http-generic-filter(#<process www.google.com<1>> "\n0\n\n")
[ ... ]
url-retrieve-synchronously("https://www.google.com/calendar/dav/MY-GOOGLE-ACCOUNT/events/")
Note that after `url-http-parse-headers' the variable
`file-name-handler-alist' is set to 'nil', thus when
`auth-source-search' loads an encrypted authinfo.gpg file, it won't
automatically get decrypted.
-David
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-07-18 18:38 bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source David Engster
@ 2012-07-19 6:35 ` Michael Albinus
2012-07-19 8:55 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Michael Albinus @ 2012-07-19 6:35 UTC (permalink / raw)
To: David Engster; +Cc: 11981
David Engster <deng@randomsample.de> writes:
> However, this workaraound assumes that everything that gets afterwards
> in the `let'-section does not depend on file name handlers. However,
> this is not the case: `url-http-handle-authentication' is called which
> in turn calls the auth-source package. This package however supports
> encrypted authinfo files which have to be automatically decrypted
> through `auto-encryption-mode'. This, of course, works through file name
> handlers.
Does it work, if Tramp is disabled instead?
--8<---------------cut here---------------start------------->8---
*** /usr/local/src/emacs/lisp/url/url-http.el.~109155~ 2012-07-19
08:29:53.339768264 +0200
--- /usr/local/src/emacs/lisp/url/url-http.el 2012-07-19
08:28:26.428326961 +0200
***************
*** 510,518 ****
;; 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)
(when (url-use-cookies url-http-target-url)
--- 510,517 ----
;; 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 Tramp, therefore.
! (tramp-mode nil))
(setq class (/ url-http-response-status 100))
(url-http-debug "Parsed HTTP headers: class=%d status=%d" class url-http-response-status)
(when (url-use-cookies url-http-target-url)
--8<---------------cut here---------------end--------------->8---
> -David
Best regards, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-07-19 6:35 ` Michael Albinus
@ 2012-07-19 8:55 ` Stefan Monnier
2012-07-19 12:51 ` David Engster
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-07-19 8:55 UTC (permalink / raw)
To: Michael Albinus; +Cc: 11981, David Engster
>> However, this workaraound assumes that everything that gets afterwards
>> in the `let'-section does not depend on file name handlers. However,
>> this is not the case: `url-http-handle-authentication' is called which
>> in turn calls the auth-source package. This package however supports
>> encrypted authinfo files which have to be automatically decrypted
>> through `auto-encryption-mode'. This, of course, works through file name
>> handlers.
> Does it work, if Tramp is disabled instead?
I think I'd first like to better understand bug#6717: why do we take
a local part of a URL, let it start with / and then pass it to
file-name-directory? That sounds like a problem in itself.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-07-19 8:55 ` Stefan Monnier
@ 2012-07-19 12:51 ` David Engster
2012-07-21 14:31 ` David Engster
0 siblings, 1 reply; 11+ messages in thread
From: David Engster @ 2012-07-19 12:51 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 11981, Michael Albinus
Stefan Monnier writes:
>>> However, this workaraound assumes that everything that gets afterwards
>>> in the `let'-section does not depend on file name handlers. However,
>>> this is not the case: `url-http-handle-authentication' is called which
>>> in turn calls the auth-source package. This package however supports
>>> encrypted authinfo files which have to be automatically decrypted
>>> through `auto-encryption-mode'. This, of course, works through file name
>>> handlers.
>> Does it work, if Tramp is disabled instead?
>
> I think I'd first like to better understand bug#6717: why do we take
> a local part of a URL, let it start with / and then pass it to
> file-name-directory? That sounds like a problem in itself.
I agree. I wonder which call to `file-name-directory' threw the error in
the original report - the most likely candidate is `url-file-directory',
which should simply be rewritten to not use `file-name-directory'.
-David
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-07-19 12:51 ` David Engster
@ 2012-07-21 14:31 ` David Engster
2012-08-06 16:01 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: David Engster @ 2012-07-21 14:31 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 11981, Michael Albinus
[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]
David Engster writes:
> Stefan Monnier writes:
>>>> However, this workaraound assumes that everything that gets afterwards
>>>> in the `let'-section does not depend on file name handlers. However,
>>>> this is not the case: `url-http-handle-authentication' is called which
>
>>>> in turn calls the auth-source package. This package however supports
>>>> encrypted authinfo files which have to be automatically decrypted
>>>> through `auto-encryption-mode'. This, of course, works through file name
>>>> handlers.
>>> Does it work, if Tramp is disabled instead?
>>
>> I think I'd first like to better understand bug#6717: why do we take
>> a local part of a URL, let it start with / and then pass it to
>> file-name-directory? That sounds like a problem in itself.
>
> I agree. I wonder which call to `file-name-directory' threw the error in
> the original report - the most likely candidate is `url-file-directory',
> which should simply be rewritten to not use `file-name-directory'.
Patch attached. It even fixes another bug along the way:
`url-file-(non)directory' would not work on hexified URLs.
I'm not 100% sure this also fixes bug #6717; it might be that there's
another file-* call lurking somewhere.
-David
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: url-util-diff.patch --]
[-- Type: text/x-patch, Size: 1180 bytes --]
=== modified file 'lisp/url/url-util.el'
--- lisp/url/url-util.el 2012-07-11 23:13:41 +0000
+++ lisp/url/url-util.el 2012-07-21 14:25:59 +0000
@@ -246,18 +246,24 @@
"Return the directory part of FILE, for a URL."
(cond
((null file) "")
- ((string-match "\\?" file)
- (file-name-directory (substring file 0 (match-beginning 0))))
- (t (file-name-directory file))))
+ ((string-match "\\(\\?\\|%3[fF]\\)" file)
+ (url-file-directory (substring file 0 (match-beginning 0))))
+ ((string-match "\\(.*\\(/\\|%2[fF]\\)\\)" file)
+ (match-string 1 file))
+ (t
+ "")))
;;;###autoload
(defun url-file-nondirectory (file)
"Return the nondirectory part of FILE, for a URL."
(cond
((null file) "")
- ((string-match "\\?" file)
- (file-name-nondirectory (substring file 0 (match-beginning 0))))
- (t (file-name-nondirectory file))))
+ ((string-match "\\(\\?\\|%3[fF]\\)" file)
+ (url-file-nondirectory (substring file 0 (match-beginning 0))))
+ ((string-match ".*\\(?:/\\|%2[fF]\\)\\(.*\\)" file)
+ (match-string 1 file))
+ (t
+ "")))
;;;###autoload
(defun url-parse-query-string (query &optional downcase allow-newlines)
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-07-21 14:31 ` David Engster
@ 2012-08-06 16:01 ` Stefan Monnier
2012-08-06 17:18 ` Jan Djärv
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-08-06 16:01 UTC (permalink / raw)
To: David Engster; +Cc: 11981, Michael Albinus
> @@ -246,18 +246,24 @@
> "Return the directory part of FILE, for a URL."
> (cond
> ((null file) "")
> - ((string-match "\\?" file)
> - (file-name-directory (substring file 0 (match-beginning 0))))
> - (t (file-name-directory file))))
> + ((string-match "\\(\\?\\|%3[fF]\\)" file)
> + (url-file-directory (substring file 0 (match-beginning 0))))
Should %3F really be treated like a ? (i.e. a special char that
delimits two different parts of a URL) rather than like "a normal ?
character encoded so as not to delimit two different parts of a URL"?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-08-06 16:01 ` Stefan Monnier
@ 2012-08-06 17:18 ` Jan Djärv
2012-08-12 14:52 ` David Engster
0 siblings, 1 reply; 11+ messages in thread
From: Jan Djärv @ 2012-08-06 17:18 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 11981, Michael Albinus, David Engster
Hello.
6 aug 2012 kl. 18:01 skrev Stefan Monnier:
>> @@ -246,18 +246,24 @@
>> "Return the directory part of FILE, for a URL."
>> (cond
>> ((null file) "")
>> - ((string-match "\\?" file)
>> - (file-name-directory (substring file 0 (match-beginning 0))))
>> - (t (file-name-directory file))))
>> + ((string-match "\\(\\?\\|%3[fF]\\)" file)
>> + (url-file-directory (substring file 0 (match-beginning 0))))
>
> Should %3F really be treated like a ? (i.e. a special char that
> delimits two different parts of a URL) rather than like "a normal ?
> character encoded so as not to delimit two different parts of a URL"?
>
A normal non-delimiting ?. How else can one get a ? in to an URL?
Jan D.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-08-06 17:18 ` Jan Djärv
@ 2012-08-12 14:52 ` David Engster
2012-08-12 17:36 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: David Engster @ 2012-08-12 14:52 UTC (permalink / raw)
To: Jan Djärv; +Cc: 11981, Michael Albinus
Jan Djärv writes:
> Hello.
>
> 6 aug 2012 kl. 18:01 skrev Stefan Monnier:
>
>>> @@ -246,18 +246,24 @@
>>> "Return the directory part of FILE, for a URL."
>>> (cond
>>> ((null file) "")
>>> - ((string-match "\\?" file)
>>> - (file-name-directory (substring file 0 (match-beginning 0))))
>>> - (t (file-name-directory file))))
>>> + ((string-match "\\(\\?\\|%3[fF]\\)" file)
>>> + (url-file-directory (substring file 0 (match-beginning 0))))
>>
>> Should %3F really be treated like a ? (i.e. a special char that
>> delimits two different parts of a URL) rather than like "a normal ?
>> character encoded so as not to delimit two different parts of a URL"?
>>
>
> A normal non-delimiting ?. How else can one get a ? in to an URL?
You are both right, of course; I was overcompensating. The match for
%3[fF] must be removed.
-David
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-08-12 14:52 ` David Engster
@ 2012-08-12 17:36 ` Stefan Monnier
2012-08-13 19:23 ` David Engster
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-08-12 17:36 UTC (permalink / raw)
To: David Engster; +Cc: 11981, Michael Albinus
> You are both right, of course; I was overcompensating. The match for
> %3[fF] must be removed.
Thanks, I installed your patch with this part adjusted (as well as the
default case changed to better match file-name-(non)directory's
behavior).
Can we now re-enable file-name-handlers in url-http-parse-headers?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-08-12 17:36 ` Stefan Monnier
@ 2012-08-13 19:23 ` David Engster
2012-08-14 14:55 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: David Engster @ 2012-08-13 19:23 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 11981, Michael Albinus
Stefan Monnier writes:
>> You are both right, of course; I was overcompensating. The match for
>> %3[fF] must be removed.
>
> Thanks, I installed your patch with this part adjusted (as well as the
> default case changed to better match file-name-(non)directory's
> behavior).
> Can we now re-enable file-name-handlers in url-http-parse-headers?
I think so. I re-enabled the file-name-handlers, created a test feed
with a colon in it
http://www.randomsample.de/foo:bar.xml
and I could subscribe to it with Gnus (the original URLs in Bug #6717
gave me http 405 errors).
-David
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source
2012-08-13 19:23 ` David Engster
@ 2012-08-14 14:55 ` Stefan Monnier
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2012-08-14 14:55 UTC (permalink / raw)
To: David Engster; +Cc: 11981-done, Michael Albinus
>> Can we now re-enable file-name-handlers in url-http-parse-headers?
> I think so. I re-enabled the file-name-handlers, created a test feed
> with a colon in it
> http://www.randomsample.de/foo:bar.xml
> and I could subscribe to it with Gnus (the original URLs in Bug #6717
> gave me http 405 errors).
OK, re-enabled, then, thank you,
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-08-14 14:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-18 18:38 bug#11981: 24.1.50; url-http-parse-headers should not disable file name handlers since it breaks auth-source David Engster
2012-07-19 6:35 ` Michael Albinus
2012-07-19 8:55 ` Stefan Monnier
2012-07-19 12:51 ` David Engster
2012-07-21 14:31 ` David Engster
2012-08-06 16:01 ` Stefan Monnier
2012-08-06 17:18 ` Jan Djärv
2012-08-12 14:52 ` David Engster
2012-08-12 17:36 ` Stefan Monnier
2012-08-13 19:23 ` David Engster
2012-08-14 14:55 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).