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

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

[Katsumi Yamaoka]

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

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2876 bytes --]

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

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

[Katsumi Yamaoka]

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