bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[João Távora]

Hello maintainers,

In the latest trunk, url-retrieve-synchronously is having trouble with
some https URLS. In the Emacs -Q scratch buffer, this returns the empty string:

  (with-current-buffer (url-retrieve-synchronously
    "https://melpa.org/packages/yascroll-20150315.605.el")
    (buffer-string))

But this works perfectly

  (with-current-buffer (url-retrieve-synchronously "https://google.com/")
    (buffer-string))

  (with-current-buffer (url-retrieve-synchronously "http://google.com/")
    (buffer-string))

No ideia why it works on google and gmail.

The misbehaving propagated silently and I came across this using M-x
package-install RET ... RET, which magically compiles a 0-byte package.

I think the protocol for url-retrieve-synchornously could be
revised. Its users seem to expect it to return nil or at least a
non-buffer if something went wrong, but that's clearly not what happened
here. Perhaps it should error.

Anyway, the buffer process is failing silently. A lower-level `url-retrieve'
doesn't even call its callback.

I'll try to debug a bit further if I find the time, in the meantime
here's a report.

João

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Alexis]

João Távora <joaotavora@gmail.com> writes:

> In the latest trunk, url-retrieve-synchronously is having 
> trouble with some https URLS. In the Emacs -Q scratch buffer, 
> this returns the empty string:
>
>   (with-current-buffer (url-retrieve-synchronously 
>     "https://melpa.org/packages/yascroll-20150315.605.el") 
>     (buffer-string))

i can't reproduce this with emacs master as at commit 
d6ea6453. Could you please try with a more recent build?

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Tao Fang]

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

joaotavora@gmail.com (João Távora) writes:

> The misbehaving propagated silently and I came across this using M-x
> package-install RET ... RET, which magically compiles a 0-byte package.
I had same problem when using package-install with restricted network
connection (or bad network conditions), sometimes I got empty buffer
without any error where as it should fail.

> I think the protocol for url-retrieve-synchornously could be
> revised. Its users seem to expect it to return nil or at least a
> non-buffer if something went wrong, but that's clearly not what happened
> here. Perhaps it should error.
I agree.

So I proposed a patch for this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-url-retrieve-synchronously-signal-error-when-fail.patch --]
[-- Type: text/x-diff, Size: 1632 bytes --]

From ff85fd2a421f751626d61b73d6f268d0cc42d702 Mon Sep 17 00:00:00 2001
From: Tao Fang <fangtao0901@gmail.com>
Date: Sat, 4 Jun 2016 23:27:55 +0800
Subject: [PATCH] url-retrieve-synchronously signal error when fail

---
 lisp/url/url.el | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lisp/url/url.el b/lisp/url/url.el
index 6d710e0..1d49b10 100644
--- a/lisp/url/url.el
+++ b/lisp/url/url.el
@@ -235,12 +235,14 @@ how long to wait for a response before giving up."
 
   (let ((retrieval-done nil)
 	(start-time (current-time))
-        (asynch-buffer nil))
+        (asynch-buffer nil)
+        err)
     (setq asynch-buffer
-	  (url-retrieve url (lambda (&rest ignored)
+	  (url-retrieve url (lambda (status &rest ignored)
 			      (url-debug 'retrieval "Synchronous fetching done (%S)" (current-buffer))
 			      (setq retrieval-done t
-				    asynch-buffer (current-buffer)))
+				    asynch-buffer (current-buffer)
+                                    err (plist-get status :error)))
 			nil silent inhibit-cookies))
     (if (null asynch-buffer)
         ;; We do not need to do anything, it was a mailto or something
@@ -300,6 +302,12 @@ how long to wait for a response before giving up."
 		(delete-process proc))
               (setq proc (and (not quit-flag)
 			      (get-buffer-process asynch-buffer)))))))
+
+      ;; Before we return, check for error except 'http
+      (when (and err
+                 (not (eq (cadr err) 'http)))
+          (signal (car err) (cdr err)))
+
       asynch-buffer)))
 
 ;; url-mm-callback called from url-mm, which requires mm-decode.
-- 
2.8.3

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Noam Postavsky]

merge 23225 23503
tag 23225 + unreproducible
quit

It looks 23225 and 23503 are the same bug. I'm also unable to
reproduce this on master. Tagging as unreproducible, but would be good
to have confirmation in case this is OSX specific (I only checked on
GNU/Linux).

In GNU Emacs 25.1.50.3 (x86_64-unknown-linux-gnu, X toolkit)
 of 2016-05-30 built on zony
Repository revision: 237244bbd5ce753bcdf79634561de515bd76c687
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Chunyang Xu]

On Sun, Jun 5, 2016 at 12:23 AM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> merge 23225 23503
> tag 23225 + unreproducible
> quit
>
> It looks 23225 and 23503 are the same bug. I'm also unable to
> reproduce this on master. Tagging as unreproducible, but would be good
> to have confirmation in case this is OSX specific (I only checked on
> GNU/Linux).

I still have this problem on latest master branch, unlike this report,
ALL https urls I tried failed.  I also found the following workaround
solves it

  (advice-add 'gnutls-available-p :around #'ignore)

previously, gnutls-available-p returns t


(emacs-version)
    ⇒ "GNU Emacs 25.1.50.14 (x86_64-apple-darwin15.5.0, NS
appkit-1404.47 Version 10.11.5 (Build 15F34))
 of 2016-06-11"

emacs-repository-version
    ⇒ "c803af788d4140c7253b1d091e6f7fb63b5a214a"

> In GNU Emacs 25.1.50.3 (x86_64-unknown-linux-gnu, X toolkit)
>  of 2016-05-30 built on zony
> Repository revision: 237244bbd5ce753bcdf79634561de515bd76c687
> Windowing system distributor 'The X.Org Foundation', version 11.0.11803000

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Noam Postavsky]

On Sat, Jun 11, 2016 at 12:00 AM, Chunyang Xu <xuchunyang.me@gmail.com> wrote:
> I still have this problem on latest master branch, unlike this report,
> ALL https urls I tried failed.

Do you get any interesting error messages after applying Tao Fang's patch?

http://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-url-retrieve-synchronously-signal-error-when-fail.patch;bug=23225;att=1;msg=11

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Chunyang Xu]

On Sat, Jun 11, 2016 at 8:47 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Sat, Jun 11, 2016 at 12:00 AM, Chunyang Xu <xuchunyang.me@gmail.com> wrote:
>> I still have this problem on latest master branch, unlike this report,
>> ALL https urls I tried failed.
>
> Do you get any interesting error messages after applying Tao Fang's patch?

(display-buffer
 (url-retrieve-synchronously "https://example.com"))

Debugger entered--Lisp error: (error connection-failed "deleted
" :host "example.com" :service 443)
  signal(error (connection-failed "deleted\n" :host "example.com" :service 443))
  (progn (signal (car err) (cdr err)))
  (if (and err (not (eq (car (cdr err)) (quote http)))) (progn (signal
(car err) (cdr err))))
  (if (null asynch-buffer) nil (let ((proc (get-buffer-process
asynch-buffer))) (while (and (not retrieval-done) (or (not timeout) (<
(float-time (time-subtract ... start-time)) timeout))) (url-debug
(quote retrieval) "Spinning in url-retrieve-synchronously: %S (%S)"
retrieval-done asynch-buffer) (if (buffer-local-value (quote
url-redirect-buffer) asynch-buffer) (setq proc (get-buffer-process
(setq asynch-buffer (buffer-local-value ... asynch-buffer)))) (if (and
proc (memq (process-status proc) (quote ...)) (eq proc (or ... proc)))
(progn (delete-process proc) (setq retrieval-done t))) (if (or
(condition-case nil (let ... ...) (quit ... ...)) (null proc)) nil (if
quit-flag (progn (delete-process proc))) (setq proc (and (not
quit-flag) (get-buffer-process asynch-buffer))))))) (if (and err (not
(eq (car (cdr err)) (quote http)))) (progn (signal (car err) (cdr
err)))) asynch-buffer)
  (let ((retrieval-done nil) (start-time (current-time))
(asynch-buffer nil) err) (setq asynch-buffer (url-retrieve url
(function (lambda (status &rest ignored) (url-debug (quote retrieval)
"Synchronous fetching done (%S)" (current-buffer)) (setq
retrieval-done t asynch-buffer (current-buffer) err (plist-get status
:error)))) nil silent inhibit-cookies)) (if (null asynch-buffer) nil
(let ((proc (get-buffer-process asynch-buffer))) (while (and (not
retrieval-done) (or (not timeout) (< (float-time ...) timeout)))
(url-debug (quote retrieval) "Spinning in url-retrieve-synchronously:
%S (%S)" retrieval-done asynch-buffer) (if (buffer-local-value (quote
url-redirect-buffer) asynch-buffer) (setq proc (get-buffer-process
(setq asynch-buffer ...))) (if (and proc (memq ... ...) (eq proc ...))
(progn (delete-process proc) (setq retrieval-done t))) (if (or
(condition-case nil ... ...) (null proc)) nil (if quit-flag (progn
...)) (setq proc (and ... ...)))))) (if (and err (not (eq (car (cdr
err)) (quote http)))) (progn (signal (car err) (cdr err))))
asynch-buffer))
  url-retrieve-synchronously("https://example.com")
  (display-buffer (url-retrieve-synchronously "https://example.com"))
  eval((display-buffer (url-retrieve-synchronously "https://example.com")) nil)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)


> http://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-url-retrieve-synchronously-signal-error-when-fail.patch;bug=23225;att=1;msg=11

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Noam Postavsky]

On Sat, Jun 11, 2016 at 8:54 AM, Chunyang Xu <xuchunyang.me@gmail.com> wrote:
> On Sat, Jun 11, 2016 at 8:47 PM, Noam Postavsky
> <npostavs@users.sourceforge.net> wrote:
>> Do you get any interesting error messages after applying Tao Fang's patch?
>
> (display-buffer
>  (url-retrieve-synchronously "https://example.com"))
>
> Debugger entered--Lisp error: (error connection-failed "deleted
> " :host "example.com" :service 443)
>   signal(error (connection-failed "deleted\n" :host "example.com" :service 443))

"deleted\n"? That's weird. What do you get in *Messages* after
increasing gnutls-log-level?

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Chunyang Xu]

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

On Sat, Jun 11, 2016 at 9:36 PM, Noam Postavsky <
npostavs@users.sourceforge.net> wrote:
> On Sat, Jun 11, 2016 at 8:54 AM, Chunyang Xu <xuchunyang.me@gmail.com>
wrote:
>> On Sat, Jun 11, 2016 at 8:47 PM, Noam Postavsky
>> <npostavs@users.sourceforge.net> wrote:
>>> Do you get any interesting error messages after applying Tao Fang's
patch?
>>
>> (display-buffer
>>  (url-retrieve-synchronously "https://example.com"))
>>
>> Debugger entered--Lisp error: (error connection-failed "deleted
>> " :host "example.com" :service 443)
>>   signal(error (connection-failed "deleted\n" :host "example.com"
:service 443))
>
> "deleted\n"? That's weird. What do you get in *Messages* after
> increasing gnutls-log-level?

after setting it to 2


Contacting host: example.com:443
gnutls.c: [1] (Emacs) connecting to host: example.com
gnutls.c: [1] (Emacs) allocating credentials
gnutls.c: [2] (Emacs) allocating x509 credentials
gnutls.c: [2] (Emacs) using default verification flags
gnutls.c: [audit] There was a non-CA certificate in the trusted list:
O=Entrust.net,OU=www.entrust.net/CPS_2048 incorp. by ref. (limits
liab.),OU=(c) 1999 Entrust.net Limited,CN=Entrust.net Certification
Authority (2048).

gnutls.c: [1] (Emacs) gnutls callbacks
gnutls.c: [1] (Emacs) gnutls_init
gnutls.c: [1] (Emacs) got non-default priority string: NORMAL
gnutls.c: [1] (Emacs) setting the priority string
gnutls.c: [audit] Note that the security level of the Diffie-Hellman key
exchange has been lowered to 256 bits and this may allow decryption of the
session data

gnutls.c: [2] WRITE: -1 returned from 0x12, errno: 57

gnutls.c: [1] (Emacs) fatal error: Error in the push function.
gnutls.c: [2] WRITE: -1 returned from 0x12, errno: 57

gnutls.c: [1] (Emacs) fatal error: Error in the push function.
gnutls.c: [2] WRITE: -1 returned from 0x12, errno: 57

gnutls.c: [1] (Emacs) fatal error: Error in the push function.
gnutls.c: [2] WRITE: -1 returned from 0x12, errno: 57

gnutls.c: [1] (Emacs) fatal error: Error in the push function.
gnutls.c: [1] (Emacs) fatal error: The specified session has been
invalidated for some reason. [5998 times]
gnutls.c: [2] (Emacs) Deallocating x509 credentials
progn: peculiar error: "deleted
", :host, "example.com", :service, 443

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

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Tao Fang]

  I've tested on macOS Sierra 10.12(16A254g)/Xcode 7.3.1(7D1014), emacs
(compiled from git master --with-gnutls) and gnutls-3.5.2 (compiled from
source), and same problem here.
  
> gnutls.c: [2] WRITE: -1 returned from 0x12, errno: 57
  errno 57 is ENOTCONN (defined in /usr/include/sys/errno.h) in the case
of that after called connect() with asynchronous socket, when TCP
three-way handshake was not accomplished, calling write() will return
ENOTCONN. I'm not sure if this conform POSIX.1 since the manual(man 2
write) mentions standards and AFAIK [EAGAIN] or [EWOULDBLOCK] would be a
better choice.
  And gnutls didn't properly handle error 57(ENOTCONN) in errno_to_gerr()
(lib/buffers.c), I've fired a bug report with above message at:
  https://gitlab.com/gnutls/gnutls/issues/117

  Back to emacs, and after some searching and compiling test, it seems
that this commit cause the master branch broken on OS X:

    63efcc268635dea78c6bd80749eae4ee2c72d717
    Author:     John Wiegley <johnw@newartisans.com>
    AuthorDate: Fri Mar 11 13:33:32 2016 -0800
    Commit:     John Wiegley <johnw@newartisans.com>
    CommitDate: Fri Mar 11 13:33:32 2016 -0800
    
    Parent:     b4fbd69 ; Merge from origin/emacs-25
    Parent:     facb5e2 Update Emacs manual section related to character
    folding
    Containing: master
    Follows:    emacs-25.0.92 (3006)

this problem doesn't exist before that, and current emacs-25 branch is
also good.

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Tao Fang]

Update: issue resolved, refer to bug#23982:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23982

by commit:
    1a8d31123698ccf6f165e49fcfe16631d07a7aea
    Author:     Paul Eggert <eggert@cs.ucla.edu>
    AuthorDate: Wed Aug 3 01:54:20 2016 -0700
    Commit:     Paul Eggert <eggert@cs.ucla.edu>
    CommitDate: Wed Aug 3 01:55:49 2016 -0700
    
    Parent:     64edd14 Require GnuTLS 2.12.2 or later
    Containing: master
    Follows:    emacs-25.0.92 (4312)
    
    Fix non-blocking GnuTLS with slow connection
    
    Although the problem is reported for OS X (Bug#23982), it seems to
    be possible on other POSIXish platforms.
    * src/gnutls.c (emacs_gnutls_nonblock_errno) [!WINDOWSNT]:
    New function.
    (emacs_gnutls_handshake) [!WINDOWSNT]:
    Use it as the errno function, if non-blocking.
    (Fgnutls_boot): Use GNUTLS_NONBLOCK if non-blocking.

I've tested on macOS and confirm it's been fixed.

bug#23225: 25.1.50; url-retrieve-synchronously having trouble with some https URLs

[Noam Postavsky]

forcemerge 23982 23225
quit

On Sat, Aug 6, 2016 at 7:56 PM, Tao Fang <fangtao0901@gmail.com> wrote:
>
> Update: issue resolved, refer to bug#23982:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23982
>
> by commit:
>     1a8d31123698ccf6f165e49fcfe16631d07a7aea
[...]
>     Fix non-blocking GnuTLS with slow connection
>
>     Although the problem is reported for OS X (Bug#23982), it seems to
>     be possible on other POSIXish platforms.
[...]
>
> I've tested on macOS and confirm it's been fixed.

Thanks for testing. I guess that means 23982 is macOS specific after
all (or at least the bug happens to manifest only on macOS)