unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#42382: 26.3; url-http handling of Location redirection headers containing whitespace
@ 2020-07-15 20:40 Daniele Nicolodi
  2020-07-16 16:08 ` Robert Pluim
  2020-07-19 19:17 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 8+ messages in thread
From: Daniele Nicolodi @ 2020-07-15 20:40 UTC (permalink / raw)
  To: 42382

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

url-http.el interprets HTTP responses in url-http-parse-headers. This
function contains the following code:

	 (when redirect-uri
	   ;; Clean off any whitespace and/or <...> cruft.
	   (if (string-match "\\([^ \t]+\\)[ \t]" redirect-uri)
	       (setq redirect-uri (match-string 1 redirect-uri)))
	   (if (string-match "^<\\(.*\\)>$" redirect-uri)
	       (setq redirect-uri (match-string 1 redirect-uri)))

which truncates the value of the Location header at the first whitespace
character and removes surrounding angle brackets quoting.

In RFC 7231 the Location header is defined to carry a URI-reference.
According to RFC 3986 it should be percent-encoded and thus should not
contain spaces. However, there are HTTP server implementation (notably
nginx) that do not do that. While this is a bug in those HTTP server
implementations, I think Emacs should follow what most other HTTP client
implementatios (all the ones I tested) and use the content of the
Location header unmodified. Stripping of angle bracket quotes is
unnecessary as they are not valid according to the RFCs.

Also, accordingly to the RFCs, the location header may contain a
relative location. Thus the comment that suggest that such a response is
a bug in the server should be reworded.

The attached patches implement the proposed changes.

Thank you.

[-- Attachment #2: 0001-url-http-Fix-handling-of-redirect-locations.patch --]
[-- Type: text/plain, Size: 1493 bytes --]

From 052a9934380fdd7517b5bf38aca37aaf951a644e Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <daniele@grinta.net>
Date: Fri, 10 Jul 2020 12:16:01 -0600
Subject: [PATCH 1/2] url-http: Fix handling of redirect locations

Do not break the redirect Location header when it contain spaces. In
RFC 7231 the Location header is defined to carry a URI-reference.
According to RFC 3986 it should be percent-encoded and thus should not
contain spaces. However, there are HTTP server implementation (notably
nginx) that do not do that. This makes Emacs url-http.el behave like
most other HTTP client implementatios. Also remove the stripping of
angle bracket quotes as they are not valid according to the RFCs.
---
 lisp/url/url-http.el | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 669c24571f..a746be8475 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -702,12 +702,6 @@ should be shown to the user."
 	    ;; Treat everything like '300'
 	    nil))
 	 (when redirect-uri
-	   ;; Clean off any whitespace and/or <...> cruft.
-	   (if (string-match "\\([^ \t]+\\)[ \t]" redirect-uri)
-	       (setq redirect-uri (match-string 1 redirect-uri)))
-	   (if (string-match "^<\\(.*\\)>$" redirect-uri)
-	       (setq redirect-uri (match-string 1 redirect-uri)))
-
 	   ;; Some stupid sites (like sourceforge) send a
 	   ;; non-fully-qualified URL (ie: /), which royally confuses
 	   ;; the URL library.
-- 
2.24.3 (Apple Git-128)


[-- Attachment #3: 0002-url-http-Do-not-suggest-a-broken-HTTP-server-impleme.patch --]
[-- Type: text/plain, Size: 1083 bytes --]

From 8da3326f64aaeb47f4306a4bacc3067d4bc38141 Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <daniele@grinta.net>
Date: Fri, 10 Jul 2020 18:50:39 -0600
Subject: [PATCH 2/2] url-http: Do not suggest a broken HTTP server
 implementation

Relative URIs are allowed in the Location header by the relvant RFCs.
---
 lisp/url/url-http.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index a746be8475..e0c925e13f 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -702,9 +702,7 @@ should be shown to the user."
 	    ;; Treat everything like '300'
 	    nil))
 	 (when redirect-uri
-	   ;; Some stupid sites (like sourceforge) send a
-	   ;; non-fully-qualified URL (ie: /), which royally confuses
-	   ;; the URL library.
+	   ;; Handle relative redirect URIs.
 	   (if (not (string-match url-nonrelative-link redirect-uri))
                ;; Be careful to use the real target URL, otherwise we may
                ;; compute the redirection relative to the URL of the proxy.
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-20 14:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 20:40 bug#42382: 26.3; url-http handling of Location redirection headers containing whitespace Daniele Nicolodi
2020-07-16 16:08 ` Robert Pluim
2020-07-16 16:30   ` Daniele Nicolodi
2020-07-16 17:10     ` Robert Pluim
2020-07-17  0:01       ` Lars Ingebrigtsen
2020-07-17 15:47       ` Daniele Nicolodi
2020-07-19 19:17 ` Lars Ingebrigtsen
2020-07-20 14:46   ` Daniele Nicolodi

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