Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Support clj-time Intervals/DateTime for :max-age and :expires #62

Merged
merged 1 commit into from

2 participants

@KushalP

This fixes #55.

Each commit appears with a header and body description for what I've done.

Here's a quick summary:

  1. clj-time is a dependency for ring-core
  2. Tests check that :max-age and :expires accept an integer (as originally outlined)
  3. :max-age now accepts an Interval — this is exercised through tests
  4. :expires now accepts a DateTime — exercised through tests
  5. Pre-conditions make sure that only those two keys take their respective object kinds

Let me know if any further work needs to be done.

@weavejester
Collaborator

Thanks for the work you've put into this so far! There are a few issues though...

The :expires attribute should not be a Unix timestamp, but in a format like "Wdy, DD Mon YYYY HH:MM:SS GMT". I believe the cookie spec gives some leeway, and as far as I can tell, the :rfc822 format type that clj-time has built in should create a cookie-compatible date string.

I think it would be something like:

(unparse (formatters :rfc822) value)

The valid-attr? function could be refactored a little by writing the cond as part of the and, e.g.

(and (contains? set-cookie-attrs key)
     (not (.contains (str value) ";")
     (case key
       :max-age (or (instance? Interval value) (integer? value))
       :expires (or (instance? DateTime value) (string? value))
       true))

Finally, I think your branch is a little out of date, as GitHub is telling me no automatic merging. You might want to rebase it off master.

@KushalP KushalP Add clj-time Intervals/DateTime for :max-age and :expires (fixes #55)
- Add a test for :max-age and :expires to (wrap-cookies ...). There were
  no real tests to exercise the base input cases (int, string) which the
  comment block states. This test just makes sure it fulfils that
  contract.
- clj-time is now a project dependency
- :max-age accepts an Interval as input. Updated (write-attr-map ...) to
  accept an Interval (from JodaTime) as well as an int. The interface
  from clj-time is used.
- :expires accepts a DateTime object. It converts the DateTime object in
  the equivalent RFC822 which the cookie spec requires.
- Added pre-conditions for :max-age and :expires to make sure that they
  only accept Interval and DateTime, respectively.
75c90aa
@KushalP

The commits have now been squashed into a single commit, rather than the intermediate steps.

@weavejester weavejester merged commit 86a0442 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 19, 2012
  1. @KushalP

    Add clj-time Intervals/DateTime for :max-age and :expires (fixes #55)

    KushalP authored
    - Add a test for :max-age and :expires to (wrap-cookies ...). There were
      no real tests to exercise the base input cases (int, string) which the
      comment block states. This test just makes sure it fulfils that
      contract.
    - clj-time is now a project dependency
    - :max-age accepts an Interval as input. Updated (write-attr-map ...) to
      accept an Interval (from JodaTime) as well as an int. The interface
      from clj-time is used.
    - :expires accepts a DateTime object. It converts the DateTime object in
      the equivalent RFC822 which the cookie spec requires.
    - Added pre-conditions for :max-age and :expires to make sure that they
      only accept Interval and DateTime, respectively.
This page is out of date. Refresh to see the latest.
View
3  ring-core/project.clj
@@ -5,6 +5,7 @@
[commons-codec "1.6"]
[commons-io "2.1"]
[commons-fileupload "1.2.1"]
- [javax.servlet/servlet-api "2.5"]]
+ [javax.servlet/servlet-api "2.5"]
+ [clj-time "0.3.7"]]
:profiles
{:dev {:dependencies [[org.clojure/clojure-contrib "1.2.0"]]}})
View
19 ring-core/src/ring/middleware/cookies.clj
@@ -1,6 +1,9 @@
(ns ring.middleware.cookies
"Cookie manipulation."
- (:require [ring.util.codec :as codec]))
+ (:require [ring.util.codec :as codec])
+ (:use [clj-time.core :only (in-secs)]
+ [clj-time.format :only (formatters unparse)])
+ (:import (org.joda.time Interval DateTime)))
(def ^{:private true
:doc "HTTP token: 1*<any CHAR except CTLs or tspecials>. See RFC2068"}
@@ -91,7 +94,11 @@
"Is the attribute valid?"
[[key value]]
(and (contains? set-cookie-attrs key)
- (not (.contains (str value) ";"))))
+ (not (.contains (str value) ";"))
+ (case key
+ :max-age (or (instance? Interval value) (integer? value))
+ :expires (or (instance? DateTime value) (string? value))
+ true)))
(defn- write-attr-map
"Write a map of cookie attributes to a string."
@@ -100,9 +107,11 @@
(for [[key value] attrs]
(let [attr-name (name (set-cookie-attrs key))]
(cond
- (true? value) (str ";" attr-name)
- (false? value) ""
- :else (str ";" attr-name "=" value)))))
+ (instance? Interval value) (str ";" attr-name "=" (in-secs value))
+ (instance? DateTime value) (str ";" attr-name "=" (unparse (formatters :rfc822) value))
+ (true? value) (str ";" attr-name)
+ (false? value) ""
+ :else (str ";" attr-name "=" value)))))
(defn- write-cookies
"Turn a map of cookies into a seq of strings for a Set-Cookie header."
View
57 ring-core/test/ring/middleware/test/cookies.clj
@@ -1,6 +1,7 @@
(ns ring.middleware.test.cookies
(:use clojure.test
- ring.middleware.cookies))
+ ring.middleware.cookies
+ [clj-time.core :only (interval date-time)]))
(deftest wrap-cookies-basic-cookie
(let [req {:headers {"cookie" "a=b"}}
@@ -88,3 +89,57 @@
(let [response {:cookies {"a" {:value "foo" :invalid true}}}
handler (wrap-cookies (constantly response))]
(is (thrown? AssertionError (handler {})))))
+
+(deftest wrap-cookies-accepts-max-age
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :max-age 123}}
+ handler (constantly {:cookies cookies})
+ resp ((wrap-cookies handler) {})]
+ (is (= {"Set-Cookie" (list "a=b;Path=/;Secure;HttpOnly;Max-Age=123")}
+ (:headers resp)))))
+
+(deftest wrap-cookies-accepts-expires
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :expires "123"}}
+ handler (constantly {:cookies cookies})
+ resp ((wrap-cookies handler) {})]
+ (is (= {"Set-Cookie" (list "a=b;Path=/;Secure;HttpOnly;Expires=123")}
+ (:headers resp)))))
+
+(deftest wrap-cookies-accepts-max-age-from-clj-time
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :max-age (interval (date-time 2012)
+ (date-time 2015))}}
+ handler (constantly {:cookies cookies})
+ resp ((wrap-cookies handler) {})
+ max-age 94694400]
+ (is (= {"Set-Cookie" (list (str "a=b;Path=/;Secure;HttpOnly;Max-Age=" max-age))}
+ (:headers resp)))))
+
+(deftest wrap-cookies-accepts-expires-from-clj-time
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :expires (date-time 2015 12 31)}}
+ handler (constantly {:cookies cookies})
+ resp ((wrap-cookies handler) {})
+ expires "Thu, 31 Dec 2015 00:00:00 +0000"]
+ (is (= {"Set-Cookie" (list (str "a=b;Path=/;Secure;HttpOnly;Expires=" expires))}
+ (:headers resp)))))
+
+(deftest wrap-cookies-throws-exception-when-not-using-intervals-correctly
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :expires (interval (date-time 2012)
+ (date-time 2015))}}
+ handler (constantly {:cookies cookies})]
+ (is (thrown? AssertionError ((wrap-cookies handler) {})))))
+
+(deftest wrap-cookies-throws-exception-when-not-using-datetime-correctly
+ (let [cookies {"a" {:value "b", :path "/",
+ :secure true, :http-only true,
+ :max-age (date-time 2015 12 31)}}
+ handler (constantly {:cookies cookies})]
+ (is (thrown? AssertionError ((wrap-cookies handler) {})))))
Something went wrong with that request. Please try again.