Subject: bug#29282: closed (Re: bug#29282: 26.0.90; url-cookie.el: a cookie handling bug)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 13 Nov 2017 23:59:02 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
29282: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=29282
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#29282: 26.0.90; url-cookie.el: a cookie handling bug
From: Katsumi Yamaoka <[email-masked]>
To: [email-masked]
Date: Tue, 14 Nov 2017 08:57:51 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

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.
[...]

Subject: 26.0.90; url-cookie.el: a cookie handling bug
From: Katsumi Yamaoka <[email-masked]>
To: [email-masked]
Date: Mon, 13 Nov 2017 17:43:52 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

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.

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