Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation on `get-cookie-policy` not correct? #444

Closed
andersenleo opened this issue Apr 5, 2018 · 5 comments
Closed

Documentation on `get-cookie-policy` not correct? #444

andersenleo opened this issue Apr 5, 2018 · 5 comments

Comments

@andersenleo
Copy link

@andersenleo andersenleo commented Apr 5, 2018

I'm accessing the Datadog REST API and get an error due to "Invalid cookie header":

Apr 05, 2018 8:40:25 AM org.apache.http.client.protocol.ResponseProcessCookies processCookies
WARNING: Invalid cookie header: "Set-Cookie: DD-PSHARD=3; expires="Thu, 12-Apr-2018 06:40:25 GMT"; Max-Age=604800; Path=/; secure; HttpOnly". Invalid 'expires' attribute: "Thu, 12-Apr-2018 06:40:25 GMT"

I've worked around this by removing the wrap-cookies middleware for the request like this:

(http/with-middleware (remove #(= % clj-http.cookies/wrap-cookies) http/default-middleware) .... ).

I still get the warning though.

I was under the impression that I could declare a (defmethod get-cookie-policy ..) as described in the README, but this fails with an error in request-config where (get-cookie-policy ..) is expected to return a String.

@dakrone

This comment has been minimized.

Copy link
Owner

@dakrone dakrone commented Apr 6, 2018

@andersenleo as for the "Invalid 'expires' attribute" error, can you try setting {:cookie-policy :standard} in the request options and see if that works for you? This sounds related to #325.

I was under the impression that I could declare a (defmethod get-cookie-policy ..) as described in the README, but this fails with an error in request-config where (get-cookie-policy ..) is expected to return a String.

You're right, the documentation here is incorrect. Currently the get-cookie-policy method returns a String that will be used for lookup for the cookie spec. Let me do some research about a way to fix this so it is possible to override (though the above option should work for you for now hopefully)

@andersenleo

This comment has been minimized.

Copy link
Author

@andersenleo andersenleo commented Apr 6, 2018

(Note: This is on clj-http 3.8.0)

If I use the :standard cookie-policy with my code like this (with the middleware hack in place):

(defn dd-metric
  "Query Datadog for a metric"
  ([{:keys [api-key application-key] :as datadog-credentials} query from to]
   (:body
    ;; Hack to avoid parsing invalid cookies. 
    (http/with-middleware (remove #(= % clj-http.cookies/wrap-cookies) http/default-middleware)
      (http/get "https://app.datadoghq.com/api/v1/query"
                {:cookie-policy :standard
                 :as            :json
                 :query-params  {"api_key"         api-key
                                 "application_key" application-key
                                 "from"            from
                                 "to"              to
                                 "query"           query}}))))

  ([datadog-credentials query]
   (let [now  (utc-now)
         from (utc-minus now 60)]
     (dd-metric datadog-credentials query (dt->sec from) (dt->sec now)))))

I no longer get the warning printed on STDERR 👍

But if I leave out the middleware hack, I still get an error:

1. Unhandled org.apache.http.cookie.MalformedCookieException
   Invalid 'expires' attribute: "Fri, 13-Apr-2018 05:59:55 GMT"

  BasicExpiresHandler.java:   64  org.apache.http.impl.cookie.BasicExpiresHandler/parse
    BrowserCompatSpec.java:  172  org.apache.http.impl.cookie.BrowserCompatSpec/parse
               cookies.clj:   72  clj-http.cookies/decode-cookie
               cookies.clj:   64  clj-http.cookies/decode-cookie
                  core.clj: 2644  clojure.core/map/fn
              LazySeq.java:   40  clojure.lang.LazySeq/sval
              LazySeq.java:   49  clojure.lang.LazySeq/seq
                   RT.java:  521  clojure.lang.RT/seq
                  core.clj:  137  clojure.core/seq
             protocols.clj:   24  clojure.core.protocols/seq-reduce
             protocols.clj:   75  clojure.core.protocols/fn
             protocols.clj:   75  clojure.core.protocols/fn
             protocols.clj:   13  clojure.core.protocols/fn/G
                  core.clj: 6545  clojure.core/reduce
                  core.clj: 6527  clojure.core/reduce
               cookies.clj:   85  clj-http.cookies/decode-cookies
               cookies.clj:   82  clj-http.cookies/decode-cookies
               cookies.clj:   93  clj-http.cookies/decode-cookie-header
               cookies.clj:   88  clj-http.cookies/decode-cookie-header
               cookies.clj:  122  clj-http.cookies/cookies-response
               cookies.clj:  118  clj-http.cookies/cookies-response
               cookies.clj:  131  clj-http.cookies/wrap-cookies/fn
                 links.clj:   63  clj-http.links/wrap-links/fn
                client.clj: 1041  clj-http.client/wrap-unknown-host/fn
                client.clj: 1214  clj-http.client/request*
                client.clj: 1207  clj-http.client/request*
                client.clj: 1220  clj-http.client/get
                client.clj: 1216  clj-http.client/get
               RestFn.java:  423  clojure.lang.RestFn/invoke
                      REPL:   18  retcoord.metrics/dd-metric
@dakrone

This comment has been minimized.

Copy link
Owner

@dakrone dakrone commented Apr 11, 2018

For now, since you don't need the cookies, can you set :decode-cookies false in the request? that should prevent the middleware from trying to parse the cookies.

I'm still looking into the other aspects, this is a complex part of the apache client!

dakrone added a commit that referenced this issue Apr 11, 2018
This adds two advanced options for specifying the `CookieSpec` that Apache
should use when parsing/validating cookies.

Relates to #444
dakrone added a commit that referenced this issue Apr 11, 2018
This adds two advanced options for specifying the `CookieSpec` that Apache
should use when parsing/validating cookies.

Relates to #444
@andersenleo

This comment has been minimized.

Copy link
Author

@andersenleo andersenleo commented Apr 11, 2018

Ah -- why didn't I see that config option before? :-).

So, with {:cookie-policy :standard, :decode-cookies false} I no longer get that warning 👍.

@dakrone

This comment has been minimized.

Copy link
Owner

@dakrone dakrone commented Apr 25, 2018

Okay, glad to hear that, for now then, I'm going to resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.