unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* URL not following some 302 redirects after recent changes
@ 2006-11-22  2:35 Diane Murray
  2007-01-20 21:30 ` Chong Yidong
  2007-01-31 13:44 ` Diane Murray
  0 siblings, 2 replies; 20+ messages in thread
From: Diane Murray @ 2006-11-22  2:35 UTC (permalink / raw)


Sometime after 2006-10-26 URL redirects stopped working correctly
(Emacs CVS of 2006-09-19 and 2006-10-26 works, 2006-10-31 and
2006-11-19 don't work), perhaps due to changes made in revision 1.36
of url-http.el.

For example, <http://www.cliki.net/cliki> returns the following
headers, but `url-retrieve' runs the callback function instead of
first retrieving the new location:

HTTP/1.0 302 Redirected
Date: Fri, 17 Nov 2006 17:50:59 GMT
Server: Araneida/0.84
Connection: close
Content-Type: text/html
Last-Modified: Fri, 17 Nov 2006 17:50:59 GMT
Location: http://www.cliki.net/CLiki
Pragma: no-cache
Expires: Fri, 30 Oct 1998 14:19:41 GMT

The above-mentioned working versions retrieve the redirected URL
correctly.

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

* Re: URL not following some 302 redirects after recent changes
  2006-11-22  2:35 URL not following some 302 redirects after recent changes Diane Murray
@ 2007-01-20 21:30 ` Chong Yidong
  2007-01-22  6:04   ` Richard Stallman
                     ` (2 more replies)
  2007-01-31 13:44 ` Diane Murray
  1 sibling, 3 replies; 20+ messages in thread
From: Chong Yidong @ 2007-01-20 21:30 UTC (permalink / raw)


Diane Murray <disumu@x3y2z1.net> writes:

> Sometime after 2006-10-26 URL redirects stopped working correctly
> (Emacs CVS of 2006-09-19 and 2006-10-26 works, 2006-10-31 and
> 2006-11-19 don't work), perhaps due to changes made in revision 1.36
> of url-http.el.
>
> For example, <http://www.cliki.net/cliki> returns the following
> headers, but `url-retrieve' runs the callback function instead of
> first retrieving the new location:
>
> HTTP/1.0 302 Redirected
> Date: Fri, 17 Nov 2006 17:50:59 GMT
> Server: Araneida/0.84
> Connection: close
> Content-Type: text/html
> Last-Modified: Fri, 17 Nov 2006 17:50:59 GMT
> Location: http://www.cliki.net/CLiki
> Pragma: no-cache
> Expires: Fri, 30 Oct 1998 14:19:41 GMT
>
> The above-mentioned working versions retrieve the redirected URL
> correctly.

This seems to be the original intention.  There is a comment in
url-http.el:

    ;; 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.

So it appears that it is up to the url client to resolve a 302
redirect manually.

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-20 21:30 ` Chong Yidong
@ 2007-01-22  6:04   ` Richard Stallman
  2007-01-22 15:02     ` Stefan Monnier
  2007-01-24 16:05   ` Stefan Monnier
  2007-02-12  1:37   ` T. V. Raman
  2 siblings, 1 reply; 20+ messages in thread
From: Richard Stallman @ 2007-01-22  6:04 UTC (permalink / raw)
  Cc: emacs-devel

    So it appears that it is up to the url client to resolve a 302
    redirect manually.

I think this is a change we made in URL in the past year; perhaps in
the past month.  It ought to be in NEWS.  Is it?

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-22  6:04   ` Richard Stallman
@ 2007-01-22 15:02     ` Stefan Monnier
  2007-01-23  2:07       ` Richard Stallman
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2007-01-22 15:02 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

>     So it appears that it is up to the url client to resolve a 302
>     redirect manually.

> I think this is a change we made in URL in the past year; perhaps in
> the past month.  It ought to be in NEWS.  Is it?

URL was not part of Emacs last time we made a release, so it's not clear
what is "NEWS".


        Stefan

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-22 15:02     ` Stefan Monnier
@ 2007-01-23  2:07       ` Richard Stallman
  0 siblings, 0 replies; 20+ messages in thread
From: Richard Stallman @ 2007-01-23  2:07 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    URL was not part of Emacs last time we made a release, so it's not clear
    what is "NEWS".

That is a good point.  We should mention this change in URL in NEWS
even though URL wasn't in Emacs before.  Would you like to do that?

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-20 21:30 ` Chong Yidong
  2007-01-22  6:04   ` Richard Stallman
@ 2007-01-24 16:05   ` Stefan Monnier
  2007-02-12  1:37   ` T. V. Raman
  2 siblings, 0 replies; 20+ messages in thread
From: Stefan Monnier @ 2007-01-24 16:05 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel

>> Sometime after 2006-10-26 URL redirects stopped working correctly
>> (Emacs CVS of 2006-09-19 and 2006-10-26 works, 2006-10-31 and
>> 2006-11-19 don't work), perhaps due to changes made in revision 1.36
>> of url-http.el.
>> 
>> For example, <http://www.cliki.net/cliki> returns the following
>> headers, but `url-retrieve' runs the callback function instead of
>> first retrieving the new location:
>> 
>> HTTP/1.0 302 Redirected
>> Date: Fri, 17 Nov 2006 17:50:59 GMT
>> Server: Araneida/0.84
>> Connection: close
>> Content-Type: text/html
>> Last-Modified: Fri, 17 Nov 2006 17:50:59 GMT
>> Location: http://www.cliki.net/CLiki
>> Pragma: no-cache
>> Expires: Fri, 30 Oct 1998 14:19:41 GMT
>> 
>> The above-mentioned working versions retrieve the redirected URL
>> correctly.

> This seems to be the original intention.  There is a comment in
> url-http.el:

>     ;; 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.

> So it appears that it is up to the url client to resolve a 302
> redirect manually.

I'm not sure I understand: the comment only talks about requests other than
GET or HEAD.  From what I can tell the OP's request was a GET, so the
comment shouldn't apply, right?


        Stefan "trying to understand what's going on so as to write
                something useful in the NEWS file"

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

* Re: URL not following some 302 redirects after recent changes
  2006-11-22  2:35 URL not following some 302 redirects after recent changes Diane Murray
  2007-01-20 21:30 ` Chong Yidong
@ 2007-01-31 13:44 ` Diane Murray
  2007-02-01  0:21   ` Chong Yidong
  1 sibling, 1 reply; 20+ messages in thread
From: Diane Murray @ 2007-01-31 13:44 UTC (permalink / raw)
  To: emacs-devel

On Wed, 22 Nov 2006 03:35:17 +0100 I wrote:
> Sometime after 2006-10-26 URL redirects stopped working correctly
> (Emacs CVS of 2006-09-19 and 2006-10-26 works, 2006-10-31 and
> 2006-11-19 don't work), perhaps due to changes made in revision 1.36
> of url-http.el.

It seems that every time the redirects aren't working is when servers
using HTTP/1.0 send a "Connection: close" header in their 302 response
- 302 responses from servers using HTTP/1.1 work, even when they send
"Connection: close".  Some sites where URL does not redirect correctly
are <http://www.cliki.net> (as mentioned in my original email),
<http://livejournal.com>, and <http://fsf.org>.  Here's an example
with fsf.org.

URL requests:

  GET / HTTP/1.1 
  MIME-Version: 1.0 
  Connection: keep-alive 
  Extension: Security/Digest Security/SSL 
  Host: fsf.org 

The server at fsf.org, using HTTP/1.0, returns the following.  The
connection status for the process changes from "open" to "connection
broken by remote peer ", so `url-http-async-sentinel' activates the
callback.

  HTTP/1.0 302 Moved Temporarily
  Date: Sat, 27 Jan 2007 17:20:20 GMT
  Server: Apache/1.3.33 (Debian GNU/Linux) mod_python/2.7.10 Python/2.3.5 PHP/4.4.0-3 mod_ssl/2.8.24 OpenSSL/0.9.7g
  Location: http://www.fsf.org/
  Content-Type: text/html; charset=iso-8859-1
  X-Cache: MISS from www.fsf.org
  X-Cache-Lookup: MISS from www.fsf.org:80
  Connection: close
  ...

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-31 13:44 ` Diane Murray
@ 2007-02-01  0:21   ` Chong Yidong
  2007-02-02 11:26     ` Richard Stallman
  0 siblings, 1 reply; 20+ messages in thread
From: Chong Yidong @ 2007-02-01  0:21 UTC (permalink / raw)
  To: Diane Murray, Magnus Henoch; +Cc: emacs-devel

Diane Murray <disumu@x3y2z1.net> writes:

> On Wed, 22 Nov 2006 03:35:17 +0100 I wrote:
>> Sometime after 2006-10-26 URL redirects stopped working correctly
>> (Emacs CVS of 2006-09-19 and 2006-10-26 works, 2006-10-31 and
>> 2006-11-19 don't work), perhaps due to changes made in revision 1.36
>> of url-http.el.

I think I know the problem.

In url-http-async-sentinel, before sending the http request, the code
attempts to change the process sentinel to
url-http-end-of-document-sentinel, which is supposed to parse the
headers when the connection is closed:

 (cond
   ((string= (substring why 0 4) "open")
    (set-process-sentinel proc 'url-http-end-of-document-sentinel)
    (process-send-string proc (url-http-create-request)))

However, attempting to change the process sentinel in the sentinel
itself has no effect (see the elisp manual).  That's why the headers
don't get parsed when the connection is closed.

The reason HTTP 1.1 does not have this problem is that it reports the
total length of the document, so the headers get parsed as soon as we
realise, in url-http-content-length-after-change-function, that we are
at the end of the document.  For HTTP 1.0, we have to wait till the
connection is closed before realizing that we are at the end of the
document.

A quick fix is to introduce a variable url-http-catch-end-of-document,
and have url-http-async-sentinel call
url-http-end-of-document-sentinel instead of doing its usual job if
that is non-nil.  Then url-http-async-sentinel can set
url-http-catch-end-of-document instead of trying to change the process
sentinel.

The patch below implements this.  I will commit it if there are no
objections in the next couple of days.

*** emacs/lisp/url/url-http.el.~1.49.~	2007-01-21 08:38:56.000000000 -0500
--- emacs/lisp/url/url-http.el	2007-01-31 19:12:53.000000000 -0500
***************
*** 30,35 ****
--- 30,36 ----
  (defvar url-http-extra-headers)
  (defvar url-http-target-url)
  (defvar url-http-proxy)
+ (defvar url-http-catch-end-of-document)
  (require 'url-gw)
  (require 'url-util)
  (require 'url-parse)
***************
*** 1118,1123 ****
--- 1119,1125 ----
  		       url-http-extra-headers
  		       url-http-data
  		       url-http-target-url
+ 		       url-http-catch-end-of-document
  		       url-http-proxy))
  	  (set (make-local-variable var) nil))
  
***************
*** 1132,1137 ****
--- 1134,1140 ----
  	      url-callback-arguments cbargs
  	      url-http-after-change-function 'url-http-wait-for-headers-change-function
  	      url-http-target-url url-current-object
+ 	      url-http-catch-end-of-document nil
  	      url-http-proxy url-using-proxy)
  
  	(set-process-buffer connection buffer)
***************
*** 1140,1145 ****
--- 1143,1149 ----
  	  (cond
  	   ((eq status 'connect)
  	    ;; Asynchronous connection
+ 	    (setq url-http-catch-end-of-document nil)
  	    (set-process-sentinel connection 'url-http-async-sentinel))
  	   ((eq status 'failed)
  	    ;; Asynchronous connection failed
***************
*** 1153,1170 ****
    (declare (special url-callback-arguments))
    ;; We are performing an asynchronous connection, and a status change
    ;; has occurred.
!   (with-current-buffer (process-buffer proc)
!     (cond
!      ((string= (substring why 0 4) "open")
!       (set-process-sentinel proc 'url-http-end-of-document-sentinel)
!       (process-send-string proc (url-http-create-request)))
!      (t
!       (setf (car url-callback-arguments)
! 	    (nconc (list :error (list 'error 'connection-failed why
! 				      :host (url-host (or url-http-proxy url-current-object))
! 				      :service (url-port (or url-http-proxy url-current-object))))
! 		   (car url-callback-arguments)))
!       (url-http-activate-callback)))))
  
  ;; Since Emacs 19/20 does not allow you to change the
  ;; `after-change-functions' hook in the midst of running them, we fake
--- 1157,1176 ----
    (declare (special url-callback-arguments))
    ;; We are performing an asynchronous connection, and a status change
    ;; has occurred.
!   (if url-http-catch-end-of-document
!       (url-http-end-of-document-sentinel proc why)
!     (with-current-buffer (process-buffer proc)
!       (cond
!        ((string= (substring why 0 4) "open")
! 	(setq url-http-catch-end-of-document t)
! 	(process-send-string proc (url-http-create-request)))
!        (t
! 	(setf (car url-callback-arguments)
! 	      (nconc (list :error (list 'error 'connection-failed why
! 					:host (url-host (or url-http-proxy url-current-object))
! 					:service (url-port (or url-http-proxy url-current-object))))
! 		     (car url-callback-arguments)))
! 	(url-http-activate-callback))))))
  
  ;; Since Emacs 19/20 does not allow you to change the
  ;; `after-change-functions' hook in the midst of running them, we fake

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-01  0:21   ` Chong Yidong
@ 2007-02-02 11:26     ` Richard Stallman
  2007-02-02 15:37       ` Stefan Monnier
  2007-02-02 17:09       ` Chong Yidong
  0 siblings, 2 replies; 20+ messages in thread
From: Richard Stallman @ 2007-02-02 11:26 UTC (permalink / raw)
  To: Chong Yidong; +Cc: mange, disumu, emacs-devel

      While a sentinel is running, the process sentinel is temporarily
    set to @code{nil} so that the sentinel won't run recursively.
    For this reason it is not possible for a sentinel to specify
    a new sentinel.

We did that to avoid a class of bugs.  But it has caused a new
problem.  So I am looking for a better solution.

Here's one idea.  Instead of setting the sentinel temporarily to nil,
set it temporarily to `sentinel-temporarily-inhibited'.  That would be
ignored just as nil is ignored.

On restoring the sentinel, if its current value isn't
`sentinel-temporarily-inhibited', just discard the old value instead
of restoring it.

This would prevent recursion just like the current code, but sentinels
that set the sentinel would work once again.

Does anyone see a problem with this fix?

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 11:26     ` Richard Stallman
@ 2007-02-02 15:37       ` Stefan Monnier
  2007-02-02 15:45         ` David Kastrup
  2007-02-03 11:19         ` Richard Stallman
  2007-02-02 17:09       ` Chong Yidong
  1 sibling, 2 replies; 20+ messages in thread
From: Stefan Monnier @ 2007-02-02 15:37 UTC (permalink / raw)
  To: rms; +Cc: Chong Yidong, mange, disumu, emacs-devel

> Here's one idea.  Instead of setting the sentinel temporarily to nil,
> set it temporarily to `sentinel-temporarily-inhibited'.  That would be
> ignored just as nil is ignored.

> On restoring the sentinel, if its current value isn't
> `sentinel-temporarily-inhibited', just discard the old value instead
> of restoring it.

Why can't we do the same thing with nil?


        Stefan

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 15:37       ` Stefan Monnier
@ 2007-02-02 15:45         ` David Kastrup
  2007-02-03 11:19           ` Richard Stallman
  2007-02-03 11:19         ` Richard Stallman
  1 sibling, 1 reply; 20+ messages in thread
From: David Kastrup @ 2007-02-02 15:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Chong Yidong, mange, rms, disumu, emacs-devel

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

>> Here's one idea.  Instead of setting the sentinel temporarily to nil,
>> set it temporarily to `sentinel-temporarily-inhibited'.  That would be
>> ignored just as nil is ignored.
>
>> On restoring the sentinel, if its current value isn't
>> `sentinel-temporarily-inhibited', just discard the old value instead
>> of restoring it.
>
> Why can't we do the same thing with nil?

If people are allowed to reseat the sentinel to arbitrary functions in
the sentinel, they would be surprised to find out that they can't
equally well clear it.

Instead of using `sentinel-temporarily-inhibited' I'd probably prefer
`ignore'.  We have similar behavior for filter functions.

-- 
David Kastrup

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 11:26     ` Richard Stallman
  2007-02-02 15:37       ` Stefan Monnier
@ 2007-02-02 17:09       ` Chong Yidong
  2007-02-03 11:19         ` Richard Stallman
  2007-02-22  1:38         ` Kim F. Storm
  1 sibling, 2 replies; 20+ messages in thread
From: Chong Yidong @ 2007-02-02 17:09 UTC (permalink / raw)
  To: rms; +Cc: mange, disumu, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Here's one idea.  Instead of setting the sentinel temporarily to nil,
> set it temporarily to `sentinel-temporarily-inhibited'.  That would be
> ignored just as nil is ignored.
>
> On restoring the sentinel, if its current value isn't
> `sentinel-temporarily-inhibited', just discard the old value instead
> of restoring it.
>
> This would prevent recursion just like the current code, but sentinels
> that set the sentinel would work once again.
>
> Does anyone see a problem with this fix?

Only that it's a rather deep change for the current stage of the
release (especially considering that the current behavior of sentinels
has been in place since Emacs 21), but it's your call.  If you like, I
can check in the following patch, which implements this idea (plus the
appropriate doc updates).  I have verified that it solves the bug too.

*** emacs/src/process.c.~1.498.~	2007-01-21 08:39:11.000000000 -0500
--- emacs/src/process.c	2007-02-02 12:04:42.000000000 -0500
***************
*** 152,157 ****
--- 152,158 ----
  Lisp_Object Qrun, Qstop, Qsignal;
  Lisp_Object Qopen, Qclosed, Qconnect, Qfailed, Qlisten;
  Lisp_Object Qlocal, Qipv4, Qdatagram;
+ Lisp_Object Qsentinel_inhibited;
  #ifdef AF_INET6
  Lisp_Object Qipv6;
  #endif
***************
*** 6554,6560 ****
  exec_sentinel_unwind (data)
       Lisp_Object data;
  {
!   XPROCESS (XCAR (data))->sentinel = XCDR (data);
    return Qnil;
  }
  
--- 6555,6563 ----
  exec_sentinel_unwind (data)
       Lisp_Object data;
  {
!   if (EQ (XPROCESS (XCAR (data))->sentinel,
! 	  Qsentinel_inhibited))
!     XPROCESS (XCAR (data))->sentinel = XCDR (data);
    return Qnil;
  }
  
***************
*** 6592,6600 ****
    if (NILP (sentinel))
      return;
  
!   /* Zilch the sentinel while it's running, to avoid recursive invocations;
!      assure that it gets restored no matter how the sentinel exits.  */
!   p->sentinel = Qnil;
    record_unwind_protect (exec_sentinel_unwind, Fcons (proc, sentinel));
    /* Inhibit quit so that random quits don't screw up a running filter.  */
    specbind (Qinhibit_quit, Qt);
--- 6595,6604 ----
    if (NILP (sentinel))
      return;
  
!   /* Set the sentinel to Qsentinel_inhibited while it's running, to
!      avoid recursive invocations.  It gets restored when the sentinel
!      exits, unless a new sentinel has been set.  */
!   p->sentinel = Qsentinel_inhibited;
    record_unwind_protect (exec_sentinel_unwind, Fcons (proc, sentinel));
    /* Inhibit quit so that random quits don't screw up a running filter.  */
    specbind (Qinhibit_quit, Qt);
***************
*** 7031,7036 ****
--- 7035,7042 ----
    staticpro (&Qlisten);
    Qlocal = intern ("local");
    staticpro (&Qlocal);
+   Qsentinel_inhibited = intern ("sentinel-inhibited");
+   staticpro (&Qsentinel_inhibited);
    Qipv4 = intern ("ipv4");
    staticpro (&Qipv4);
  #ifdef AF_INET6

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 15:37       ` Stefan Monnier
  2007-02-02 15:45         ` David Kastrup
@ 2007-02-03 11:19         ` Richard Stallman
  1 sibling, 0 replies; 20+ messages in thread
From: Richard Stallman @ 2007-02-03 11:19 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: cyd, mange, disumu, emacs-devel

    > Here's one idea.  Instead of setting the sentinel temporarily to nil,
    > set it temporarily to `sentinel-temporarily-inhibited'.  That would be
    > ignored just as nil is ignored.

    > On restoring the sentinel, if its current value isn't
    > `sentinel-temporarily-inhibited', just discard the old value instead
    > of restoring it.

    Why can't we do the same thing with nil?

nil as the value of the sentinal has a meaning already.
It can't mean "do restore the saved value".
What if a sentinel sets the sentinel to nil
meaning "turn off the sentinel"?

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 15:45         ` David Kastrup
@ 2007-02-03 11:19           ` Richard Stallman
  2007-02-03 11:42             ` David Kastrup
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Stallman @ 2007-02-03 11:19 UTC (permalink / raw)
  To: David Kastrup; +Cc: cyd, mange, monnier, disumu, emacs-devel

    Instead of using `sentinel-temporarily-inhibited' I'd probably prefer
    `ignore'.

`ignore' is probably not a useful filter function, but it is not
absurd.  I would rather use a special symbol which doesn't mean
anything else.  Or some other object that is meaningless as a sentinel,
such as the number 0.

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 17:09       ` Chong Yidong
@ 2007-02-03 11:19         ` Richard Stallman
  2007-02-22  1:38         ` Kim F. Storm
  1 sibling, 0 replies; 20+ messages in thread
From: Richard Stallman @ 2007-02-03 11:19 UTC (permalink / raw)
  To: Chong Yidong; +Cc: mange, disumu, emacs-devel

Your patch looks good.

I just wonder now whether an interger such as 0 would be a better
choice than a symbol such as `sentinel-inhibited'.  We could tell
people not to define `sentinel-inhibited' as a sentinel function, but
if we use 0, we don't need to tell people not to define it as a
function, because it isn't possible to do so.

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-03 11:19           ` Richard Stallman
@ 2007-02-03 11:42             ` David Kastrup
  0 siblings, 0 replies; 20+ messages in thread
From: David Kastrup @ 2007-02-03 11:42 UTC (permalink / raw)
  To: rms; +Cc: cyd, mange, monnier, disumu, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     Instead of using `sentinel-temporarily-inhibited' I'd probably prefer
>     `ignore'.
>
> `ignore' is probably not a useful filter function, but it is not
> absurd.  I would rather use a special symbol which doesn't mean
> anything else.  Or some other object that is meaningless as a sentinel,
> such as the number 0.

One problem is that you have to actually check such a value before
running it.  And if nobody was tempted to run it, there would be no
necessity of storing a different value in the sentinel in the first
place.

I would prefer to be able to get a useful error message if a bad value
ends up in a process sentinel.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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

* Re: URL not following some 302 redirects after recent changes
  2007-01-20 21:30 ` Chong Yidong
  2007-01-22  6:04   ` Richard Stallman
  2007-01-24 16:05   ` Stefan Monnier
@ 2007-02-12  1:37   ` T. V. Raman
  2 siblings, 0 replies; 20+ messages in thread
From: T. V. Raman @ 2007-02-12  1:37 UTC (permalink / raw)
  To: cyd; +Cc: emacs-devel


It would still be nice to have a custom option that tells url to
repeatedly follow redirects -- a la curl's --location and
--location-trusted flags.

>>>>> "Chong" == Chong Yidong <cyd@stupidchicken.com> writes:
    Chong> Diane Murray <disumu@x3y2z1.net> writes:
    >> Sometime after 2006-10-26 URL redirects stopped working
    >> correctly (Emacs CVS of 2006-09-19 and 2006-10-26 works,
    >> 2006-10-31 and 2006-11-19 don't work), perhaps due to
    >> changes made in revision 1.36 of url-http.el.
    >> 
    >> For example, <http://www.cliki.net/cliki> returns the
    >> following headers, but `url-retrieve' runs the callback
    >> function instead of first retrieving the new location:
    >> 
    >> HTTP/1.0 302 Redirected Date: Fri, 17 Nov 2006 17:50:59
    >> GMT Server: Araneida/0.84 Connection: close Content-Type:
    >> text/html Last-Modified: Fri, 17 Nov 2006 17:50:59 GMT
    >> Location: http://www.cliki.net/CLiki Pragma: no-cache
    >> Expires: Fri, 30 Oct 1998 14:19:41 GMT
    >> 
    >> The above-mentioned working versions retrieve the
    >> redirected URL correctly.
    Chong> 
    Chong> This seems to be the original intention.  There is a
    Chong> comment in url-http.el:
    Chong> 
    Chong>     ;; If the 301|302 status code is received in
    Chong> response to a ;; request other than GET or HEAD, the
    Chong> user agent MUST NOT ;; automatically redirect the
    Chong> request unless it can be ;; confirmed by the user,
    Chong> since this might change the ;; conditions under which
    Chong> the request was issued.
    Chong> 
    Chong> So it appears that it is up to the url client to
    Chong> resolve a 302 redirect manually.
    Chong> 
    Chong> 
    Chong> 
    Chong> _______________________________________________
    Chong> Emacs-devel mailing list Emacs-devel@gnu.org
    Chong> http://lists.gnu.org/mailman/listinfo/emacs-devel

-- 
Best Regards,
--raman

      
Email:  raman@users.sf.net
WWW:    http://emacspeak.sf.net/raman/
AIM:    emacspeak       GTalk: tv.raman.tv@gmail.com
PGP:    http://emacspeak.sf.net/raman/raman-almaden.asc
Google: tv+raman 
IRC:    irc://irc.freenode.net/#emacs

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-02 17:09       ` Chong Yidong
  2007-02-03 11:19         ` Richard Stallman
@ 2007-02-22  1:38         ` Kim F. Storm
  2007-02-22  1:43           ` Chong Yidong
  1 sibling, 1 reply; 20+ messages in thread
From: Kim F. Storm @ 2007-02-22  1:38 UTC (permalink / raw)
  To: Chong Yidong; +Cc: mange, rms, disumu, emacs-devel


What happened to the following patch (+ RMS' suggestion to use
an integer instead of the sentinel-inhibited symbol) ??

I've added the bug to FOR-RELEASE.



Chong Yidong <cyd@stupidchicken.com> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>> Here's one idea.  Instead of setting the sentinel temporarily to nil,
>> set it temporarily to `sentinel-temporarily-inhibited'.  That would be
>> ignored just as nil is ignored.
>>
>> On restoring the sentinel, if its current value isn't
>> `sentinel-temporarily-inhibited', just discard the old value instead
>> of restoring it.
>>
>> This would prevent recursion just like the current code, but sentinels
>> that set the sentinel would work once again.
>>
>> Does anyone see a problem with this fix?
>
> Only that it's a rather deep change for the current stage of the
> release (especially considering that the current behavior of sentinels
> has been in place since Emacs 21), but it's your call.  If you like, I
> can check in the following patch, which implements this idea (plus the
> appropriate doc updates).  I have verified that it solves the bug too.
>
> *** emacs/src/process.c.~1.498.~	2007-01-21 08:39:11.000000000 -0500
> --- emacs/src/process.c	2007-02-02 12:04:42.000000000 -0500
> ***************
> *** 152,157 ****
> --- 152,158 ----
>   Lisp_Object Qrun, Qstop, Qsignal;
>   Lisp_Object Qopen, Qclosed, Qconnect, Qfailed, Qlisten;
>   Lisp_Object Qlocal, Qipv4, Qdatagram;
> + Lisp_Object Qsentinel_inhibited;
>   #ifdef AF_INET6
>   Lisp_Object Qipv6;
>   #endif
> ***************
> *** 6554,6560 ****
>   exec_sentinel_unwind (data)
>        Lisp_Object data;
>   {
> !   XPROCESS (XCAR (data))->sentinel = XCDR (data);
>     return Qnil;
>   }
>   
> --- 6555,6563 ----
>   exec_sentinel_unwind (data)
>        Lisp_Object data;
>   {
> !   if (EQ (XPROCESS (XCAR (data))->sentinel,
> ! 	  Qsentinel_inhibited))
> !     XPROCESS (XCAR (data))->sentinel = XCDR (data);
>     return Qnil;
>   }
>   
> ***************
> *** 6592,6600 ****
>     if (NILP (sentinel))
>       return;
>   
> !   /* Zilch the sentinel while it's running, to avoid recursive invocations;
> !      assure that it gets restored no matter how the sentinel exits.  */
> !   p->sentinel = Qnil;
>     record_unwind_protect (exec_sentinel_unwind, Fcons (proc, sentinel));
>     /* Inhibit quit so that random quits don't screw up a running filter.  */
>     specbind (Qinhibit_quit, Qt);
> --- 6595,6604 ----
>     if (NILP (sentinel))
>       return;
>   
> !   /* Set the sentinel to Qsentinel_inhibited while it's running, to
> !      avoid recursive invocations.  It gets restored when the sentinel
> !      exits, unless a new sentinel has been set.  */
> !   p->sentinel = Qsentinel_inhibited;
>     record_unwind_protect (exec_sentinel_unwind, Fcons (proc, sentinel));
>     /* Inhibit quit so that random quits don't screw up a running filter.  */
>     specbind (Qinhibit_quit, Qt);
> ***************
> *** 7031,7036 ****
> --- 7035,7042 ----
>     staticpro (&Qlisten);
>     Qlocal = intern ("local");
>     staticpro (&Qlocal);
> +   Qsentinel_inhibited = intern ("sentinel-inhibited");
> +   staticpro (&Qsentinel_inhibited);
>     Qipv4 = intern ("ipv4");
>     staticpro (&Qipv4);
>   #ifdef AF_INET6

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-22  1:38         ` Kim F. Storm
@ 2007-02-22  1:43           ` Chong Yidong
  2007-02-22  9:26             ` Kim F. Storm
  0 siblings, 1 reply; 20+ messages in thread
From: Chong Yidong @ 2007-02-22  1:43 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: mange, rms, disumu, emacs-devel

storm@cua.dk (Kim F. Storm) writes:

> What happened to the following patch (+ RMS' suggestion to use
> an integer instead of the sentinel-inhibited symbol) ??
>
> I've added the bug to FOR-RELEASE.

I requested delaying this issue until after the release, since a small
easy effective fix for url-http.el was available (and is already
installed).  RMS said OK.

We are getting too close to the release to be doing this kind of
tinkering, IMO.

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

* Re: URL not following some 302 redirects after recent changes
  2007-02-22  1:43           ` Chong Yidong
@ 2007-02-22  9:26             ` Kim F. Storm
  0 siblings, 0 replies; 20+ messages in thread
From: Kim F. Storm @ 2007-02-22  9:26 UTC (permalink / raw)
  To: Chong Yidong; +Cc: mange, rms, disumu, emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> What happened to the following patch (+ RMS' suggestion to use
>> an integer instead of the sentinel-inhibited symbol) ??
>>
>> I've added the bug to FOR-RELEASE.
>
> I requested delaying this issue until after the release, since a small
> easy effective fix for url-http.el was available (and is already
> installed).  RMS said OK.

That's good - thanks.

>
> We are getting too close to the release to be doing this kind of
> tinkering, IMO.

Yes.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

end of thread, other threads:[~2007-02-22  9:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-22  2:35 URL not following some 302 redirects after recent changes Diane Murray
2007-01-20 21:30 ` Chong Yidong
2007-01-22  6:04   ` Richard Stallman
2007-01-22 15:02     ` Stefan Monnier
2007-01-23  2:07       ` Richard Stallman
2007-01-24 16:05   ` Stefan Monnier
2007-02-12  1:37   ` T. V. Raman
2007-01-31 13:44 ` Diane Murray
2007-02-01  0:21   ` Chong Yidong
2007-02-02 11:26     ` Richard Stallman
2007-02-02 15:37       ` Stefan Monnier
2007-02-02 15:45         ` David Kastrup
2007-02-03 11:19           ` Richard Stallman
2007-02-03 11:42             ` David Kastrup
2007-02-03 11:19         ` Richard Stallman
2007-02-02 17:09       ` Chong Yidong
2007-02-03 11:19         ` Richard Stallman
2007-02-22  1:38         ` Kim F. Storm
2007-02-22  1:43           ` Chong Yidong
2007-02-22  9:26             ` Kim F. Storm

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