all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* url-retrieve-synchronously randomly fails on https URLs (patch included)
@ 2007-10-27 10:47 Riccardo Murri
  2007-10-27 23:41 ` Richard Stallman
  0 siblings, 1 reply; 19+ messages in thread
From: Riccardo Murri @ 2007-10-27 10:47 UTC (permalink / raw)
  To: bug-gnu-emacs


To trigger the error just try a few times::

  (url-retrieve-synchronously "https://www.google.com/")


Error backtrace::

  Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
    url-http-wait-for-headers-change-function(1 199 198)
    url-http-generic-filter(#<process www.google.com> "EF344D7E1950942DBA877D79DDD8100A21A2592D03A210A4109\n    Key-Arg   : None\n    Start Time: 1193480981\n    Timeout   : 300 (sec)\n    Verify return code: 20 (unable to get local issuer certificate)\n---\n")
    accept-process-output(#<process www.google.com>)
    byte-code("=8c2\x18=8c3	!)=87" [inhibit-quit proc nil accept-process-output] 2)
    url-retrieve-synchronously("https://www.google.com/")
    (kill-buffer (url-retrieve-synchronously "https://www.google.com/"))
    eval((kill-buffer (url-retrieve-synchronously "https://www.google.com/")))
    eval-last-sexp-1(t)
    eval-last-sexp(t)
    eval-print-last-sexp()
    call-interactively(eval-print-last-sexp)


Sample contents of the URL retrieve buffer after a failed request::

  EF344D7E1950942DBA877D79DDD8100A21A2592D03A210A4109
      Key-Arg   : None
      Start Time: 1193480981
      Timeout   : 300 (sec)
      Verify return code: 20 (unable to get local issuer certificate)
  ---
  HTTP/1.1 302 Found

  Location: http://www.google.com

  Date: Sat, 27 Oct 2007 10:29:41 GMT

  Content-Type: text/html; charset=UTF-8

  Server: GFE/1.3

  Content-Length: 218

  

  <HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
  <TITLE>302 Moved</TITLE></HEAD><BODY>
  <H1>302 Moved</H1>
  The document has moved
  <A HREF="http://www.google.com">here</A>.

  </BODY></HTML>

  read:errno=0


I traced down the bug to `open-tls-stream', which returns immediately
after having seen the regexp `tls-success' in the buffer.  However,
both `openssl s_client' and `gnutls' print more information after that
line, thus `open-tls-stream' may occasionally return the buffer to the
caller when point is still amidst openssl/gnutls output.  

A solution is to have `open-tls-stream' wait a little more to accept
output; after applying the patch below I cannot trigger the error
anymore.


--- src/emacs22/lisp/net/tls.el	2007-08-05 21:06:12.000000000 +0200
+++ emacs/lisp/tls.el	2007-10-27 12:23:47.000000000 +0200
@@ -154,9 +154,14 @@
             (sit-for 1)))
 	(message "Opening TLS connection with `%s'...%s" cmd
 		 (if done "done" "failed"))
-	(if done
-	    (setq done process)
-	  (delete-process process))))
+        (if (not done)
+            (delete-process process)
+          ;; both `openssl s_client' and `gnutls' print some more info
+          ;; after the line matched by regexp `tls-success'; wait here
+          ;; a bit more, so we can be sure that point is at the end of
+          ;; it all, before handing the buffer back to caller
+          (accept-process-output process 1)
+          (setq done process))))
     (message "Opening TLS connection to `%s'...%s"
 	     host (if done "done" "failed"))
     (when use-temp-buffer


Riccardo




In GNU Emacs 22.1.1 (powerpc-unknown-linux-gnu, GTK+ Version 2.10.13)
 of 2007-08-22 on voltaire, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.70101000
configured using `configure  '--build=powerpc-linux-gnu' '--host=powerpc-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs22:/etc/emacs:/usr/local/share/emacs/22.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/22.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/22.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=powerpc-linux-gnu' 'host_alias=powerpc-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2''

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: it_IT.UTF-8
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Text

Minor modes in effect:
  tooltip-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  line-number-mode: t

Recent input:
<down> <down> <right> <right> C-SPC C-e <escape> w 
C-x 4 b <return> C-y C-j <left> <backspace> C-x C-e 
C-x C-e C-x C-e C-x C-e C-x C-e C-x C-e C-x C-e C-x 
0 <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <menu-bar> <help-menu> <report-emacs-b
ug>

Recent messages:
t
Contacting host: www.google.com:443
Reading [text/html; charset=UTF-8]... 226 bytes of 218 bytes (104%)
Reading... done.
Reading [text/html]... 227 bytes of 218 bytes (104%)
www.google.it tried to set a cookie for domain .google.it - rejected.
t
Loading emacsbug...
Loading regexp-opt...done
Loading emacsbug...done




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-27 10:47 url-retrieve-synchronously randomly fails on https URLs (patch included) Riccardo Murri
@ 2007-10-27 23:41 ` Richard Stallman
  2007-10-28 12:40   ` Riccardo Murri
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Stallman @ 2007-10-27 23:41 UTC (permalink / raw)
  To: Riccardo Murri; +Cc: emacs-devel

    I traced down the bug to `open-tls-stream', which returns immediately
    after having seen the regexp `tls-success' in the buffer.  However,
    both `openssl s_client' and `gnutls' print more information after that
    line, thus `open-tls-stream' may occasionally return the buffer to the
    caller when point is still amidst openssl/gnutls output.  

    A solution is to have `open-tls-stream' wait a little more to accept
    output; after applying the patch below I cannot trigger the error
    anymore.

+        (if (not done)
+            (delete-process process)
+          ;; both `openssl s_client' and `gnutls' print some more info
+          ;; after the line matched by regexp `tls-success'; wait here
+          ;; a bit more, so we can be sure that point is at the end of
+          ;; it all, before handing the buffer back to caller
+          (accept-process-output process 1)
+          (setq done process))))

This is not totally reliable, because it is possible for
accept-process-output to get just part of the output.
That may be unlikely, which is why you have not seen it happen,
but it is possible.

So here is another suggestion.  Can you change the `tls-success'
regexp so that it matches all the text that openssl or gnutls
would produce?  (You might need to use \(...\|...\) to match
each on separately.)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-27 23:41 ` Richard Stallman
@ 2007-10-28 12:40   ` Riccardo Murri
  2007-10-29  9:22     ` Richard Stallman
  0 siblings, 1 reply; 19+ messages in thread
From: Riccardo Murri @ 2007-10-28 12:40 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

On 10/28/07, Richard Stallman <rms@gnu.org> wrote:
>     I traced down the bug to `open-tls-stream', which returns immediately
>     after having seen the regexp `tls-success' in the buffer.  However,
>     both `openssl s_client' and `gnutls' print more information after that
>     line, thus `open-tls-stream' may occasionally return the buffer to the
>     caller when point is still amidst openssl/gnutls output.
>
>     A solution is to have `open-tls-stream' wait a little more to accept
>     output;
>     [deletia]
>
> This is not totally reliable, because it is possible for
> accept-process-output to get just part of the output.
> That may be unlikely, which is why you have not seen it happen,
> but it is possible.
>
> So here is another suggestion.  Can you change the `tls-success'
> regexp so that it matches all the text that openssl or gnutls
> would produce?  (You might need to use \(...\|...\) to match
> each on separately.)
>

No, I don't think that is feasible: `openssl s_client' is *very*
verbose and its output is meant for humans to read rather than for
programs to parse, so any regexp that matches the informational
messages has a good chance of eating up some of the data too.

I had a look at the sources, and wrote a regexp that tries to match
only the *last* informational message; the appended patch looks for
that *after* `tls-success' has been matched.

Unless I got it all wrong, this is much more complicated than the
original three-liner; I wonder if it is worth going this way instead of
a very simple patch that could still be buggy on rare occasions.


-- 
Riccardo Murri, via Galeazzo Alessi 61, 00176 Roma


--- src/emacs22/lisp/net/tls.el 2007-08-05 21:06:12.000000000 +0200
+++ emacs/lisp/tls.el   2007-10-28 13:12:11.000000000 +0100
@@ -51,6 +51,10 @@
   (autoload 'format-spec "format-spec")
   (autoload 'format-spec-make "format-spec"))

+(eval-when-compile
+  (require 'rx)  ; for writing readable regexps
+  (require 'cl)) ; for `decf'
+
 (defgroup tls nil
   "Transport Layer Security (TLS) parameters."
   :group 'comm)
@@ -89,6 +93,50 @@
   :type 'string
   :group 'tls)

+(defcustom tls-end-of-info
+ (rx
+  (or
+   ;; `openssl s_client` regexp
+   (sequence
+    ;; see ssl/ssl_txt.c lines 219--220
+    line-start
+    "    Verify return code: "
+    (one-or-more not-newline)
+    "\n"
+    ;; according apps/s_client.c line 1515 this is always the last line
+    ;; that is printed by s_client before the real data
+    "---\n")
+
+   ;; `gnutls` regexp
+   (sequence
+    ;; see src/cli.c lines 721--
+    (sequence line-start "- Simple Client Mode:\n")
+    (zero-or-more
+     (or
+      "\n" ; ignore blank lines
+      ;; XXX: we have no way of knowing if the STARTTLS handshake
+      ;; sequence has completed successfully, because `gnutls` will
+      ;; only report failure.
+      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))
+ "Regexp matching end of TLS session information; client data begins
after this.
+The default matches `openssl s_client' (version 0.9.8c) and
+`gnutls-cli' (version 2.0.1) output."
+  :version "22.1"
+  :type 'regexp
+  :group 'tls)
+
+(defcustom tls-end-of-info-match-attempts 10
+  "How many attempts to make at matching `tls-end-of-info'.
+This is intended as a safety measure to avoid a potentially
+endless loop: after this many attempts, presume `tls-end-of-info'
+regexp is broken and will never be matched, so consider that
+stream is already at the start of client data and hope for the best.
+
+Use a negative value to retry indefinitely."
+  :version "22.1"
+  :type 'integer
+  :group 'tls)
+
 (defun tls-certificate-information (der)
   "Parse X.509 certificate in DER format into an assoc list."
   (let ((certificate (concat "-----BEGIN CERTIFICATE-----\n"
@@ -130,6 +178,8 @@
        process cmd done)
     (if use-temp-buffer
        (setq buffer (generate-new-buffer " TLS")))
+    (save-excursion
+      (set-buffer buffer)
     (message "Opening TLS connection to `%s'..." host)
     (while (and (not done) (setq cmd (pop cmds)))
       (message "Opening TLS connection with `%s'..." cmd)
@@ -146,19 +196,39 @@
                              port)))))
        (while (and process
                    (memq (process-status process) '(open run))
-                   (save-excursion
-                     (set-buffer buffer) ;; XXX "blue moon" nntp.el bug
+                    (progn
                      (goto-char (point-min))
                      (not (setq done (re-search-forward tls-success nil t)))))
          (unless (accept-process-output process 1)
             (sit-for 1)))
        (message "Opening TLS connection with `%s'...%s" cmd
                 (if done "done" "failed"))
-       (if done
-           (setq done process)
-         (delete-process process))))
+        (if (not done)
+            (delete-process process)
+          ;; advance point to after all informational messages that
+          ;; `openssl s_client' and `gnutls' print
+          (lexical-let ((attempts tls-end-of-info-match-attempts)
+                        (start-of-data nil))
+            (while  (and
+                    (not (= 0 (if (> attempts 0) (decf attempts) -1)))
+                    ;; the string matching `tls-end-of-info' might come
+                    ;; in separate chunks from `accept-process-output',
+                    ;; so start the search always where `tls-success' ended
+                    (not (setq start-of-data
+                               (save-excursion
+                                 (if (re-search-forward tls-end-of-info nil t)
+                                     (match-end 0))))))
+              (unless (accept-process-output process 1)
+                (sit-for 1)))
+            (if start-of-data
+                ;; move point to start of client data
+                (goto-char start-of-data)
+              (warn
+               "`open-tls-stream': Could not match `tls-end-of-info'
regexp in %d attempts."
+               tls-end-of-info-match-attempts)))
+          (setq done process))))
     (message "Opening TLS connection to `%s'...%s"
-            host (if done "done" "failed"))
+             host (if done "done" "failed")))
     (when use-temp-buffer
       (if done (set-process-buffer process nil))
       (kill-buffer buffer))

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-28 12:40   ` Riccardo Murri
@ 2007-10-29  9:22     ` Richard Stallman
  2007-10-29 20:48       ` Riccardo Murri
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Stallman @ 2007-10-29  9:22 UTC (permalink / raw)
  To: Riccardo Murri; +Cc: emacs-devel

    Unless I got it all wrong, this is much more complicated than the
    original three-liner; I wonder if it is worth going this way instead of
    a very simple patch that could still be buggy on rare occasions.

It is a bad thing for software to be unreliable.  We should write it
so that it works every time, not just 99.9% of the time.

It is worth paying this small complexity to make it work right.
Besides, the code can be simpler.

    +          ;; advance point to after all informational messages that
    +          ;; `openssl s_client' and `gnutls' print
    +          (lexical-let ((attempts tls-end-of-info-match-attempts)
    +                        (start-of-data nil))

Why not ordinary let?

    +                    (not (= 0 (if (> attempts 0) (decf attempts) -1)))

I think there should not be a limit -- it should just keep looping
until it finds what it is looking for.

    +              (unless (accept-process-output process 1)
    +                (sit-for 1)))

I think there is no need for sit-for.  accept-process-output will wait.

    +              (warn
    +               "`open-tls-stream': Could not match `tls-end-of-info'
    regexp in %d attempts."
    +               tls-end-of-info-match-attempts)))

If you get rid of the counter, you won't need this either.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-29  9:22     ` Richard Stallman
@ 2007-10-29 20:48       ` Riccardo Murri
  2007-10-30  5:24         ` Richard Stallman
  2007-11-02 15:02         ` Richard Stallman
  0 siblings, 2 replies; 19+ messages in thread
From: Riccardo Murri @ 2007-10-29 20:48 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

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

> Besides, the code can be simpler.
>

Thanks for your suggestions; simplified patch to tls.el attached.


A few questions below:

>     +          ;; advance point to after all informational messages that
>     +          ;; `openssl s_client' and `gnutls' print
>     +          (lexical-let ((attempts tls-end-of-info-match-attempts)
>     +                        (start-of-data nil))
>
> Why not ordinary let?

Out of curiosity: why would ordinary `let' be better here?


>     +                    (not (= 0 (if (> attempts 0) (decf attempts) -1)))
>
> I think there should not be a limit -- it should just keep looping
> until it finds what it is looking for.
>

Yes, but this will result in a endless loop if the regex does not
match the program output for some reason (for instance, a new version
of `gnutls' changes the wording of the messages).

I understand that this problem may occur in many places, and that
tls.el would already loop forever if `tls-success' does not match;
but I would rather add more "watchdog" counters instead...

Anyway, I removed the `attempts' counter and related code, as per your
suggestion.

-- 
Riccardo Murri, via Galeazzo Alessi 61, 00176 Roma

[-- Attachment #2: tls.el.diff --]
[-- Type: text/plain, Size: 3750 bytes --]

--- src/emacs22/lisp/net/tls.el	2007-08-05 21:06:12.000000000 +0200
+++ emacs/lisp/tls.el	2007-10-29 19:17:33.000000000 +0100
@@ -51,6 +51,9 @@
   (autoload 'format-spec "format-spec")
   (autoload 'format-spec-make "format-spec"))
 
+(eval-when-compile
+  (require 'rx))  ; for writing readable regexps
+
 (defgroup tls nil
   "Transport Layer Security (TLS) parameters."
   :group 'comm)
@@ -89,6 +92,40 @@
   :type 'string
   :group 'tls)
 
+(defcustom tls-end-of-info
+ (rx 
+  (or
+   ;; `openssl s_client` regexp
+   (sequence
+    ;; see ssl/ssl_txt.c lines 219--220
+    line-start
+    "    Verify return code: "
+    (one-or-more not-newline)
+    "\n"
+    ;; according to apps/s_client.c line 1515 this is always the last
+    ;; line that is printed by s_client before the real data
+    "---\n")
+
+   ;; `gnutls` regexp
+   (sequence
+    ;; see src/cli.c lines 721--
+    (sequence line-start "- Simple Client Mode:\n")
+    (zero-or-more
+     (or 
+      "\n" ; ignore blank lines
+      ;; XXX: we have no way of knowing if the STARTTLS handshake
+      ;; sequence has completed successfully, because `gnutls` will
+      ;; only report failure.
+      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))
+ "Regexp matching end of TLS client informational messages.
+Client data stream begins after the last character matched by this.
+
+The default matches `openssl s_client' (version 0.9.8c) and
+`gnutls-cli' (version 2.0.1) output."
+  :version "22.1"
+  :type 'regexp
+  :group 'tls)
+
 (defun tls-certificate-information (der)
   "Parse X.509 certificate in DER format into an assoc list."
   (let ((certificate (concat "-----BEGIN CERTIFICATE-----\n"
@@ -130,6 +167,8 @@
 	process	cmd done)
     (if use-temp-buffer
 	(setq buffer (generate-new-buffer " TLS")))
+    (save-excursion
+      (set-buffer buffer)
     (message "Opening TLS connection to `%s'..." host)
     (while (and (not done) (setq cmd (pop cmds)))
       (message "Opening TLS connection with `%s'..." cmd)
@@ -146,19 +185,34 @@
 			      port)))))
 	(while (and process
 		    (memq (process-status process) '(open run))
-		    (save-excursion
-		      (set-buffer buffer) ;; XXX "blue moon" nntp.el bug
+                    (progn
 		      (goto-char (point-min))
 		      (not (setq done (re-search-forward tls-success nil t)))))
 	  (unless (accept-process-output process 1)
             (sit-for 1)))
 	(message "Opening TLS connection with `%s'...%s" cmd
 		 (if done "done" "failed"))
-	(if done
-	    (setq done process)
-	  (delete-process process))))
+        (if (not done)
+            (delete-process process)
+          ;; advance point to after all informational messages that
+          ;; `openssl s_client' and `gnutls' print
+          (let ((start-of-data nil))
+            (while 
+                (not (setq start-of-data 
+                           ;; the string matching `tls-end-of-info'
+                           ;; might come in separate chunks from
+                           ;; `accept-process-output', so start the
+                           ;; search where `tls-success' ended
+                           (save-excursion
+                             (if (re-search-forward tls-end-of-info nil t)
+                                 (match-end 0)))))
+              (accept-process-output process 1))
+            (if start-of-data
+                ;; move point to start of client data
+                (goto-char start-of-data)))
+          (setq done process))))
     (message "Opening TLS connection to `%s'...%s"
-	     host (if done "done" "failed"))
+             host (if done "done" "failed")))
     (when use-temp-buffer
       (if done (set-process-buffer process nil))
       (kill-buffer buffer))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-29 20:48       ` Riccardo Murri
@ 2007-10-30  5:24         ` Richard Stallman
  2007-10-30 10:23           ` Riccardo Murri
  2007-11-02 15:02         ` Richard Stallman
  1 sibling, 1 reply; 19+ messages in thread
From: Richard Stallman @ 2007-10-30  5:24 UTC (permalink / raw)
  To: Riccardo Murri; +Cc: emacs-devel

    Out of curiosity: why would ordinary `let' be better here?

It is better in general, because it is the usual thing.
Also it doesn't need CL.

    Yes, but this will result in a endless loop if the regex does not
    match the program output for some reason (for instance, a new version
    of `gnutls' changes the wording of the messages).

Right, but I think that is better than the chance of a failure
due to randomness.

Would you please post a change log for the patch?
Then it will be installed.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-30  5:24         ` Richard Stallman
@ 2007-10-30 10:23           ` Riccardo Murri
  0 siblings, 0 replies; 19+ messages in thread
From: Riccardo Murri @ 2007-10-30 10:23 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

On 10/30/07, Richard Stallman <rms@gnu.org> wrote:
> Would you please post a change log for the patch?
> Then it will be installed.
>

Draft changelog for the appended patch:

* (tls-end-of-info): New customization option (regexp).
* (open-tls-stream): Accept input until `tls-end-of-info' is matched.


-- 
Riccardo Murri, via Galeazzo Alessi 61, 00176 Roma


--- src/emacs22/lisp/net/tls.el 2007-08-05 21:06:12.000000000 +0200
+++ emacs/lisp/tls.el   2007-10-29 19:17:33.000000000 +0100
@@ -51,6 +51,9 @@
   (autoload 'format-spec "format-spec")
   (autoload 'format-spec-make "format-spec"))

+(eval-when-compile
+  (require 'rx))  ; for writing readable regexps
+
 (defgroup tls nil
   "Transport Layer Security (TLS) parameters."
   :group 'comm)
@@ -89,6 +92,40 @@
   :type 'string
   :group 'tls)

+(defcustom tls-end-of-info
+ (rx
+  (or
+   ;; `openssl s_client` regexp
+   (sequence
+    ;; see ssl/ssl_txt.c lines 219--220
+    line-start
+    "    Verify return code: "
+    (one-or-more not-newline)
+    "\n"
+    ;; according to apps/s_client.c line 1515 this is always the last
+    ;; line that is printed by s_client before the real data
+    "---\n")
+
+   ;; `gnutls` regexp
+   (sequence
+    ;; see src/cli.c lines 721--
+    (sequence line-start "- Simple Client Mode:\n")
+    (zero-or-more
+     (or
+      "\n" ; ignore blank lines
+      ;; XXX: we have no way of knowing if the STARTTLS handshake
+      ;; sequence has completed successfully, because `gnutls` will
+      ;; only report failure.
+      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))
+ "Regexp matching end of TLS client informational messages.
+Client data stream begins after the last character matched by this.
+
+The default matches `openssl s_client' (version 0.9.8c) and
+`gnutls-cli' (version 2.0.1) output."
+  :version "22.1"
+  :type 'regexp
+  :group 'tls)
+
 (defun tls-certificate-information (der)
   "Parse X.509 certificate in DER format into an assoc list."
   (let ((certificate (concat "-----BEGIN CERTIFICATE-----\n"
@@ -130,6 +167,8 @@
        process cmd done)
     (if use-temp-buffer
        (setq buffer (generate-new-buffer " TLS")))
+    (save-excursion
+      (set-buffer buffer)
     (message "Opening TLS connection to `%s'..." host)
     (while (and (not done) (setq cmd (pop cmds)))
       (message "Opening TLS connection with `%s'..." cmd)
@@ -146,19 +185,34 @@
                              port)))))
        (while (and process
                    (memq (process-status process) '(open run))
-                   (save-excursion
-                     (set-buffer buffer) ;; XXX "blue moon" nntp.el bug
+                    (progn
                      (goto-char (point-min))
                      (not (setq done (re-search-forward tls-success nil t)))))
          (unless (accept-process-output process 1)
             (sit-for 1)))
        (message "Opening TLS connection with `%s'...%s" cmd
                 (if done "done" "failed"))
-       (if done
-           (setq done process)
-         (delete-process process))))
+        (if (not done)
+            (delete-process process)
+          ;; advance point to after all informational messages that
+          ;; `openssl s_client' and `gnutls' print
+          (let ((start-of-data nil))
+            (while
+                (not (setq start-of-data
+                           ;; the string matching `tls-end-of-info'
+                           ;; might come in separate chunks from
+                           ;; `accept-process-output', so start the
+                           ;; search where `tls-success' ended
+                           (save-excursion
+                             (if (re-search-forward tls-end-of-info nil t)
+                                 (match-end 0)))))
+              (accept-process-output process 1))
+            (if start-of-data
+                ;; move point to start of client data
+                (goto-char start-of-data)))
+          (setq done process))))
     (message "Opening TLS connection to `%s'...%s"
-            host (if done "done" "failed"))
+             host (if done "done" "failed")))
     (when use-temp-buffer
       (if done (set-process-buffer process nil))
       (kill-buffer buffer))

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-10-29 20:48       ` Riccardo Murri
  2007-10-30  5:24         ` Richard Stallman
@ 2007-11-02 15:02         ` Richard Stallman
  2007-11-02 22:18           ` Reiner Steib
                             ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Richard Stallman @ 2007-11-02 15:02 UTC (permalink / raw)
  To: emacs-devel; +Cc: jas

Would someone please install this patch by Riccardo Murri
<riccardo.murri@gmail.com> into Emacs 22?  And then ack?

Simon, would you please add comments near the code in GNUtls that
outputs these messages, telling people to watch out for the need for
Emacs to detect the last part of the messages?



* (tls-end-of-info): New variable.
* (open-tls-stream): Keep reading input until `tls-end-of-info' is matched.


-- 
Riccardo Murri, via Galeazzo Alessi 61, 00176 Roma


--- src/emacs22/lisp/net/tls.el 2007-08-05 21:06:12.000000000 +0200
+++ emacs/lisp/tls.el   2007-10-29 19:17:33.000000000 +0100
@@ -51,6 +51,9 @@
   (autoload 'format-spec "format-spec")
   (autoload 'format-spec-make "format-spec"))

+(eval-when-compile
+  (require 'rx))  ; for writing readable regexps
+
 (defgroup tls nil
   "Transport Layer Security (TLS) parameters."
   :group 'comm)
@@ -89,6 +92,40 @@
   :type 'string
   :group 'tls)

+(defcustom tls-end-of-info
+ (rx
+  (or
+   ;; `openssl s_client` regexp
+   (sequence
+    ;; see ssl/ssl_txt.c lines 219--220
+    line-start
+    "    Verify return code: "
+    (one-or-more not-newline)
+    "\n"
+    ;; according to apps/s_client.c line 1515 this is always the last
+    ;; line that is printed by s_client before the real data
+    "---\n")
+
+   ;; `gnutls` regexp
+   (sequence
+    ;; see src/cli.c lines 721--
+    (sequence line-start "- Simple Client Mode:\n")
+    (zero-or-more
+     (or
+      "\n" ; ignore blank lines
+      ;; XXX: we have no way of knowing if the STARTTLS handshake
+      ;; sequence has completed successfully, because `gnutls` will
+      ;; only report failure.
+      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))
+ "Regexp matching end of TLS client informational messages.
+Client data stream begins after the last character matched by this.
+
+The default matches `openssl s_client' (version 0.9.8c) and
+`gnutls-cli' (version 2.0.1) output."
+  :version "22.1"
+  :type 'regexp
+  :group 'tls)
+
 (defun tls-certificate-information (der)
   "Parse X.509 certificate in DER format into an assoc list."
   (let ((certificate (concat "-----BEGIN CERTIFICATE-----\n"
@@ -130,6 +167,8 @@
        process cmd done)
     (if use-temp-buffer
        (setq buffer (generate-new-buffer " TLS")))
+    (save-excursion
+      (set-buffer buffer)
     (message "Opening TLS connection to `%s'..." host)
     (while (and (not done) (setq cmd (pop cmds)))
       (message "Opening TLS connection with `%s'..." cmd)
@@ -146,19 +185,34 @@
                              port)))))
        (while (and process
                    (memq (process-status process) '(open run))
-                   (save-excursion
-                     (set-buffer buffer) ;; XXX "blue moon" nntp.el bug
+                    (progn
                      (goto-char (point-min))
                      (not (setq done (re-search-forward tls-success nil t)))))
          (unless (accept-process-output process 1)
             (sit-for 1)))
        (message "Opening TLS connection with `%s'...%s" cmd
                 (if done "done" "failed"))
-       (if done
-           (setq done process)
-         (delete-process process))))
+        (if (not done)
+            (delete-process process)
+          ;; advance point to after all informational messages that
+          ;; `openssl s_client' and `gnutls' print
+          (let ((start-of-data nil))
+            (while
+                (not (setq start-of-data
+                           ;; the string matching `tls-end-of-info'
+                           ;; might come in separate chunks from
+                           ;; `accept-process-output', so start the
+                           ;; search where `tls-success' ended
+                           (save-excursion
+                             (if (re-search-forward tls-end-of-info nil t)
+                                 (match-end 0)))))
+              (accept-process-output process 1))
+            (if start-of-data
+                ;; move point to start of client data
+                (goto-char start-of-data)))
+          (setq done process))))
     (message "Opening TLS connection to `%s'...%s"
-            host (if done "done" "failed"))
+             host (if done "done" "failed")))
     (when use-temp-buffer
       (if done (set-process-buffer process nil))
       (kill-buffer buffer))

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 15:02         ` Richard Stallman
@ 2007-11-02 22:18           ` Reiner Steib
  2007-11-02 22:37             ` Miles Bader
  2007-11-04  1:26           ` Glenn Morris
  2007-11-05 10:26           ` Simon Josefsson
  2 siblings, 1 reply; 19+ messages in thread
From: Reiner Steib @ 2007-11-02 22:18 UTC (permalink / raw)
  To: Richard Stallman; +Cc: Riccardo Murri, jas, emacs-devel

On Fri, Nov 02 2007, Richard Stallman wrote:

> +(eval-when-compile
> +  (require 'rx))  ; for writing readable regexps
[...]
> +(defcustom tls-end-of-info
> + (rx
> +  (or
> +   ;; `openssl s_client` regexp
> +   (sequence
> +    ;; see ssl/ssl_txt.c lines 219--220
> +    line-start
> +    "    Verify return code: "
> +    (one-or-more not-newline)
> +    "\n"

Well, at least for me using rx.el doesn't make the regexp more
readable.  YMMV.

> +    (save-excursion
> +      (set-buffer buffer)

`with-current-buffer'?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 22:18           ` Reiner Steib
@ 2007-11-02 22:37             ` Miles Bader
  2007-11-02 22:50               ` Lennart Borgman (gmail)
  0 siblings, 1 reply; 19+ messages in thread
From: Miles Bader @ 2007-11-02 22:37 UTC (permalink / raw)
  To: Richard Stallman; +Cc: Riccardo Murri, jas, emacs-devel

Reiner Steib <reinersteib+gmane@imap.cc> writes:
> Well, at least for me using rx.el doesn't make the regexp more
> readable.  YMMV.

I'm not really sure I find the syntax of rx more readable per se (it
definitely makes the regexp less concise!), but it is kind of
interesting to see it, and the ability to insert comments into the
middle of the regexp actually does seem potentially quite useful!

-Miles

-- 
The car has become... an article of dress without which we feel uncertain,
unclad, and incomplete.  [Marshall McLuhan, Understanding Media, 1964]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 22:37             ` Miles Bader
@ 2007-11-02 22:50               ` Lennart Borgman (gmail)
  2007-11-03  5:48                 ` tomas
  0 siblings, 1 reply; 19+ messages in thread
From: Lennart Borgman (gmail) @ 2007-11-02 22:50 UTC (permalink / raw)
  To: Miles Bader; +Cc: Riccardo Murri, emacs-devel, Richard Stallman, jas

Miles Bader wrote:
> Reiner Steib <reinersteib+gmane@imap.cc> writes:
>> Well, at least for me using rx.el doesn't make the regexp more
>> readable.  YMMV.
> 
> I'm not really sure I find the syntax of rx more readable per se (it
> definitely makes the regexp less concise!), but it is kind of
> interesting to see it, and the ability to insert comments into the
> middle of the regexp actually does seem potentially quite useful!

rx style regexp looks like lisp. Maybe a new more readable language 
looking like regexps?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 22:50               ` Lennart Borgman (gmail)
@ 2007-11-03  5:48                 ` tomas
  2007-11-03  9:35                   ` Andreas Schwab
  0 siblings, 1 reply; 19+ messages in thread
From: tomas @ 2007-11-03  5:48 UTC (permalink / raw)
  To: Lennart Borgman (gmail)
  Cc: jas, Riccardo Murri, emacs-devel, Richard Stallman, Miles Bader

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, Nov 02, 2007 at 11:50:28PM +0100, Lennart Borgman (gmail) wrote:
> Miles Bader wrote:
> >Reiner Steib <reinersteib+gmane@imap.cc> writes:
> >>Well, at least for me using rx.el doesn't make the regexp more
> >>readable.  YMMV.
> >
> >I'm not really sure I find the syntax of rx more readable per se (it
> >definitely makes the regexp less concise!), but it is kind of
> >interesting to see it, and the ability to insert comments into the
> >middle of the regexp actually does seem potentially quite useful!
> 
> rx style regexp looks like lisp. Maybe a new more readable language 
> looking like regexps?

Oh, no :)

The cool thing about rx is that it not only *looks* like lisp, but that
it *behaves* like lisp. Macro expansion, just to name one example.

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFHLAutBcgs9XrR2kYRAnh2AJ9uEYhwxoLs7IsUH9pvxwupcanEDwCdGLdv
9Hh8Le3wio2JijRA+jZPpDc=
=xnPD
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-03  5:48                 ` tomas
@ 2007-11-03  9:35                   ` Andreas Schwab
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Schwab @ 2007-11-03  9:35 UTC (permalink / raw)
  To: tomas
  Cc: Richard Stallman, Lennart Borgman (gmail), emacs-devel,
	Riccardo Murri, jas, Miles Bader

tomas@tuxteam.de writes:

> The cool thing about rx is that it not only *looks* like lisp, but that
> it *behaves* like lisp. Macro expansion, just to name one example.

You can get all the same effect with string concatenation, without any
need of weird syntax.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 15:02         ` Richard Stallman
  2007-11-02 22:18           ` Reiner Steib
@ 2007-11-04  1:26           ` Glenn Morris
  2007-11-05 10:26           ` Simon Josefsson
  2 siblings, 0 replies; 19+ messages in thread
From: Glenn Morris @ 2007-11-04  1:26 UTC (permalink / raw)
  To: rms; +Cc: jas, emacs-devel

Richard Stallman wrote:

> Would someone please install this patch by Riccardo Murri
> <riccardo.murri@gmail.com> into Emacs 22?  And then ack?

ack

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-02 15:02         ` Richard Stallman
  2007-11-02 22:18           ` Reiner Steib
  2007-11-04  1:26           ` Glenn Morris
@ 2007-11-05 10:26           ` Simon Josefsson
  2007-11-05 15:01             ` Stefan Monnier
  2007-11-06 11:22             ` Riccardo Murri
  2 siblings, 2 replies; 19+ messages in thread
From: Simon Josefsson @ 2007-11-05 10:26 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Simon, would you please add comments near the code in GNUtls that
> outputs these messages, telling people to watch out for the need for
> Emacs to detect the last part of the messages?

Done, the GnuTLS code now contains:

  /* Warning!  Do not touch this text string, it is used by external
     programs to search for when gnutls-cli has reached this point. */
  printf ("\n- Simple Client Mode:\n\n");

etc

> +   ;; `gnutls` regexp
> +   (sequence
> +    ;; see src/cli.c lines 721--
> +    (sequence line-start "- Simple Client Mode:\n")
> +    (zero-or-more
> +     (or
> +      "\n" ; ignore blank lines
> +      ;; XXX: we have no way of knowing if the STARTTLS handshake
> +      ;; sequence has completed successfully, because `gnutls` will
> +      ;; only report failure.
> +      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))


I also installed the following patch, because I think the above comment
was incorrect.

/Simon

Index: lisp/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/lisp/ChangeLog,v
retrieving revision 1.12157
diff -u -p -r1.12157 ChangeLog
--- lisp/ChangeLog	5 Nov 2007 09:06:25 -0000	1.12157
+++ lisp/ChangeLog	5 Nov 2007 10:25:21 -0000
@@ -1,3 +1,7 @@
+2007-11-05  Simon Josefsson  <simon@josefsson.org>
+
+	* net/tls.el (tls-end-of-info): Doc fix.
+
 2007-11-05  Kenichi Handa  <handa@ni.aist.go.jp>
 
 	* international/utf-7.el (utf-7-imap): New coding system.
Index: lisp/net/tls.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/net/tls.el,v
retrieving revision 1.21
diff -u -p -r1.21 tls.el
--- lisp/net/tls.el	4 Nov 2007 01:25:49 -0000	1.21
+++ lisp/net/tls.el	5 Nov 2007 10:25:21 -0000
@@ -65,9 +65,9 @@
    ;; `gnutls' regexp. See src/cli.c lines 721-.
    "^- Simple Client Mode:\n"
    "\\(\n\\|"                           ; ignore blank lines
-   ;; XXX: We have no way of knowing if the STARTTLS handshake sequence
-   ;; has completed successfully, because `gnutls' will only report
-   ;; failure.
+   ;; According to src/cli.c lines 640-650 and 705-715 the handshake
+   ;; will start after this message.  If the handshake fails, the
+   ;; programs will abort.
    "^\\*\\*\\* Starting TLS handshake\n\\)*"
    "\\)")
   "Regexp matching end of TLS client informational messages.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-05 10:26           ` Simon Josefsson
@ 2007-11-05 15:01             ` Stefan Monnier
  2007-11-05 15:04               ` Simon Josefsson
  2007-11-06 11:22             ` Riccardo Murri
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2007-11-05 15:01 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: rms, emacs-devel

Thanks Simon,

just a nitpick:

> +   ;; According to src/cli.c lines 640-650 and 705-715 the handshake
> +   ;; will start after this message.  If the handshake fails, the
> +   ;; programs will abort.

Those line numbers will probably not last eternally, so please add some
revision information (e.g. the date).


        Stefan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-05 15:01             ` Stefan Monnier
@ 2007-11-05 15:04               ` Simon Josefsson
       [not found]                 ` <E1IpDzF-0003i4-EJ@fencepost.gnu.org>
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Josefsson @ 2007-11-05 15:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: rms, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Thanks Simon,
>
> just a nitpick:
>
>> +   ;; According to src/cli.c lines 640-650 and 705-715 the handshake
>> +   ;; will start after this message.  If the handshake fails, the
>> +   ;; programs will abort.
>
> Those line numbers will probably not last eternally, so please add some
> revision information (e.g. the date).

Good idea, done.

/Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
  2007-11-05 10:26           ` Simon Josefsson
  2007-11-05 15:01             ` Stefan Monnier
@ 2007-11-06 11:22             ` Riccardo Murri
  1 sibling, 0 replies; 19+ messages in thread
From: Riccardo Murri @ 2007-11-06 11:22 UTC (permalink / raw)
  To: emacs-devel

Il 2007-11-05, Simon Josefsson <simon@josefsson.org> ha scritto:
>> +      ;; XXX: we have no way of knowing if the STARTTLS handshake
>> +      ;; sequence has completed successfully, because `gnutls` will
>> +      ;; only report failure.
>> +      (sequence line-start "\*\*\* Starting TLS handshake\n"))))))
>
> I also installed the following patch, because I think the above comment
> was incorrect.
>

That comment's purpose was to point out that there might be cases in
which `open-tls-stream' will exit successfully, returning the buffer
to the caller (as all of `tls-success' and `tls-end-of-info' matched),
*still* the TLS connection failed.


-- 
Riccardo Murri, via Galeazzo Alessi 61, 00176 Roma

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: url-retrieve-synchronously randomly fails on https URLs (patch included)
       [not found]                 ` <E1IpDzF-0003i4-EJ@fencepost.gnu.org>
@ 2007-11-08 13:20                   ` Simon Josefsson
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Josefsson @ 2007-11-08 13:20 UTC (permalink / raw)
  To: rms, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> It might be better to say the function name instead of line numbers.
...
>     +   ;; According to src/cli.c lines 640-650 and 705-715 the handshake
>     +   ;; will start after this message.  If the handshake fails, the
>     +   ;; programs will abort.
>
> That is a good idea.  But is src/cli.c a program in GNUtls?  If so,
> please say that explicitly.  Otherwise people will think it refers to
> Emacs, and when they see there is no src/cli.c in Emacs, they will
> think it is erroneous.

I've changed it to:

   ;; According to GnuTLS v2.1.5 src/cli.c lines 640-650 and 705-715
   ;; in main() the handshake will start after this message.  If the
   ;; handshake fails, the programs will abort.

/Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2007-11-08 13:20 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-27 10:47 url-retrieve-synchronously randomly fails on https URLs (patch included) Riccardo Murri
2007-10-27 23:41 ` Richard Stallman
2007-10-28 12:40   ` Riccardo Murri
2007-10-29  9:22     ` Richard Stallman
2007-10-29 20:48       ` Riccardo Murri
2007-10-30  5:24         ` Richard Stallman
2007-10-30 10:23           ` Riccardo Murri
2007-11-02 15:02         ` Richard Stallman
2007-11-02 22:18           ` Reiner Steib
2007-11-02 22:37             ` Miles Bader
2007-11-02 22:50               ` Lennart Borgman (gmail)
2007-11-03  5:48                 ` tomas
2007-11-03  9:35                   ` Andreas Schwab
2007-11-04  1:26           ` Glenn Morris
2007-11-05 10:26           ` Simon Josefsson
2007-11-05 15:01             ` Stefan Monnier
2007-11-05 15:04               ` Simon Josefsson
     [not found]                 ` <E1IpDzF-0003i4-EJ@fencepost.gnu.org>
2007-11-08 13:20                   ` Simon Josefsson
2007-11-06 11:22             ` Riccardo Murri

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.