GNU bug report logs - #29282
26.0.90; url-cookie.el: a cookie handling bug

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29282 in the body.
You can then email your comments to 29282 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.90; url-cookie.el: a cookie handling bug
Date: Mon, 13 Nov 2017 17:43:52 +0900

[Message part 1 (text/plain, inline)]

Hi,

A cookie is fed from a web site via the Set-Cookie header like
this:

Set-Cookie: NAME=VALUE; Max-Age=-86400; Expires=Sun, 12 Nov 2017 06:26:31 GMT; Path=/; HTTPOnly

In this case, NAME and VALUE appearing in the beginning is the
cookie, and the others are its attributions.  However, url-cookie
recognizes Max-Age, HTTPOnly, etc. as individual cookies, and
sends them to the web site when a user posts forms in the page.
This will cause "500 Internal Server Error" in some web site[1].

In additin, although Max-Age should be preferred to Expires[2],
url-cookie doesn't process it.

A patch is below.

[1] Try visiting <https://help.openstreetmap.org> and
<https://help.openstreetmap.org/questions/5356/who-edited-my-map-corrections-and-made-it-all-wrong-again/5357>
in turn using eww.

To try the patched url-cookie.el, you have to delete those bogus
cookies in advance.  An easy way to do that is to shutdown Emacs
and to delete the "~/.emacs.d/url/cookies" file.

[2] <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie>


* lisp/url/url-cookie.el (url-cookie-handle-set-cookie):
Regard a Set-Cookie header as it contains a single cookie;
prefer Max-Age to Expires and convert it to Expires;
remove support for old time string styles.

[Message part 2 (text/x-patch, inline)]

--- url-cookie.el~	2017-11-09 22:09:37.790145300 +0000
+++ url-cookie.el	2017-11-13 08:41:08.487776400 +0000
@@ -249,44 +249,20 @@
 	 (current-url (url-view-url t))
 	 (trusted url-cookie-trusted-urls)
 	 (untrusted url-cookie-untrusted-urls)
-	 (expires (cdr-safe (assoc-string "expires" args t)))
+	 (max-age (cdr-safe (assoc-string "max-age" args t)))
 	 (localpart (or (cdr-safe (assoc-string "path" args t))
 			(file-name-directory
 			 (url-filename url-current-object))))
-	 (rest nil))
+	 (expires nil) (rest nil))
     (dolist (this args)
-      (or (member (downcase (car this)) '("secure" "domain" "expires" "path"))
+      (or (member (downcase (car this))
+		  '("secure" "domain" "max-age" "expires" "path"))
 	  (setq rest (cons this rest))))
-
-    ;; Sometimes we get dates that the timezone package cannot handle very
-    ;; gracefully - take care of this here, instead of in url-cookie-expired-p
-    ;; to speed things up.
-    (and expires
-	 (string-match
-	  (concat "^[^,]+, +\\(..\\)-\\(...\\)-\\(..\\) +"
-		  "\\(..:..:..\\) +\\[*\\([^]]+\\)\\]*$")
-	  expires)
-	 (setq expires (concat (match-string 1 expires) " "
-			       (match-string 2 expires) " "
-			       (match-string 3 expires) " "
-			       (match-string 4 expires) " ["
-			       (match-string 5 expires) "]")))
-
-    ;; This one is for older Emacs/XEmacs variants that don't
-    ;; understand this format without tenths of a second in it.
-    ;; Wednesday, 30-Dec-2037 16:00:00 GMT
-    ;;       - vs -
-    ;; Wednesday, 30-Dec-2037 16:00:00.00 GMT
-    (and expires
-	 (string-match
-	  "\\([0-9]+\\)-\\([A-Za-z]+\\)-\\([0-9]+\\)[ \t]+\\([0-9]+:[0-9]+:[0-9]+\\)\\(\\.[0-9]+\\)*[ \t]+\\([-+a-zA-Z0-9]+\\)"
-	  expires)
-	 (setq expires (concat (match-string 1 expires) "-"	; day
-			       (match-string 2 expires) "-"	; month
-			       (match-string 3 expires) " "	; year
-			       (match-string 4 expires) ".00 " ; hour:minutes:seconds
-			       (match-string 6 expires)))) ":" ; timezone
-
+    (if (and max-age (string-match "\\`-?[0-9]+\\'" max-age))
+	(setq expires (format-time-string "%a %b %d %H:%M:%S %Y GMT"
+					  (time-add nil (read max-age))
+					  t))
+      (setq expires (cdr-safe (assoc-string "expires" args t))))
     (while (consp trusted)
       (if (string-match (car trusted) current-url)
 	  (setq trusted (- (match-end 0) (match-beginning 0)))
@@ -322,8 +298,8 @@
       nil)
      ((url-cookie-host-can-set-p (url-host url-current-object) domain)
       ;; Cookie is accepted by the user, and passes our security checks.
-      (dolist (cur rest)
-	(url-cookie-store (car cur) (cdr cur) expires domain localpart secure)))
+      (url-cookie-store (caar rest) (cdar rest)
+			expires domain localpart secure))
      (t
       (url-lazy-message "%s tried to set a cookie for domain %s - rejected."
 			(url-host url-current-object) domain)))))

Message #10 received at 29282-done <at> debbugs.gnu.org (full text, mbox):

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: 29282-done <at> debbugs.gnu.org
Subject: Re: bug#29282: 26.0.90; url-cookie.el: a cookie handling bug
Date: Tue, 14 Nov 2017 08:57:51 +0900

I've installed the patch (slightly improved) in emacs-26.
Eww and url package users may want to remove old cookies store.

> An easy way to do that is to shutdown Emacs and to delete
> the "~/.emacs.d/url/cookies" file.

Thanks.

On Mon, 13 Nov 2017 17:43:52 +0900, Katsumi Yamaoka wrote:
> Hi,

> A cookie is fed from a web site via the Set-Cookie header like
> this:

> Set-Cookie: NAME=VALUE; Max-Age=-86400; Expires=Sun, 12 Nov 2017 06:26:31 GMT; Path=/; HTTPOnly

> In this case, NAME and VALUE appearing in the beginning is the
> cookie, and the others are its attributions.  However, url-cookie
> recognizes Max-Age, HTTPOnly, etc. as individual cookies, and
> sends them to the web site when a user posts forms in the page.
> This will cause "500 Internal Server Error" in some web site[1].

> In additin, although Max-Age should be preferred to Expires[2],
> url-cookie doesn't process it.

> A patch is below.

> [1] Try visiting <https://help.openstreetmap.org> and
> <https://help.openstreetmap.org/questions/5356/who-edited-my-map-corrections-and-made-it-all-wrong-again/5357>
> in turn using eww.

> To try the patched url-cookie.el, you have to delete those bogus
> cookies in advance.  An easy way to do that is to shutdown Emacs
> and to delete the "~/.emacs.d/url/cookies" file.

> [2] <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie>

> * lisp/url/url-cookie.el (url-cookie-handle-set-cookie):
> Regard a Set-Cookie header as it contains a single cookie;
> prefer Max-Age to Expires and convert it to Expires;
> remove support for old time string styles.
[...]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.