gfredericks / vcr-clj

Generic IO playback for clojure
117 stars 20 forks source link

Playing back cartridges with https data fails #2

Closed paulspencerwilliams closed 10 years ago

paulspencerwilliams commented 10 years ago

I've successfully used cartridge to record and playback plain http traffic. However, when I try to record and playback encrypted https traffic, (json in my case), data from the resultant playback is invalid - I believe encrypted. Certainly, the contents of the cassette file is encrypted.

gfredericks commented 10 years ago

Oh interesting! So the cassette contents is definitely encrypted, not merely a base-64 encoding of the plaintext?

gfredericks commented 10 years ago

I think figuring this out requires determining how clj-http handles https. It looks like it happens at a low level.

vcr-clj is just treating clj-http.core/request as an opaque function, recording its inputs/outputs. So the main question is how clj-http's handling of HTTPS relates to that function.

paulspencerwilliams commented 10 years ago

Had presumed the files contents where encrypted - hadn't checked. I agree with your assumption that clj-http's function would just return un-encrpyted content, but obviously we're wrong.

Can I help in any way?

gfredericks commented 10 years ago

I can't naively reproduce this. What I tried:

(ns user
  (:require [clj-http.client :as client]
            [vcr-clj.clj-http :refer [with-cassette]]))

;; first time, records actual http call
(def body1 (:body (with-cassette "foo" (client/get "https://google.com"))))

;; second time, uses cassette
(def body2 (:body (with-cassette "foo" (client/get "https://google.com"))))

;; returning true indicates to me this is working correctly
(= body1 body2)
;; => true

Can you verify that you get the same result, and describe how what you're doing is different from this?

paulspencerwilliams commented 10 years ago

Hi fredericksgary,

Firstly,

When I run your code in the repl, all three lines execute but with unexpected output if you println body1 / body2

Body1 - from actually http request shows full html (shortened in this example:

<!doctype html><html itemsco...here.

However, in body 2 has different content:

301 Moved

301 Moved

The document has moved here.

Further, the failing code in my project is at https://github.com/BlackPepperSoftware/runtothesun2014-strava-facade/blob/web-service-stub-erroring/test/ridetothesun2014_strava_facade/test/strava.clj

I believed it was an clj-vcr / https issue is that on master, the tests repeatedly pass. However, on introduction of clj-vcr, a first test iteration passes and records an entry in /cassettes. However, subsequent tests fail with a ->

Exception in thread "main" java.lang.Exception: JSON error (unexpected character): at clojure.data.json$_read.invoke(json.clj:226) at clojure.data.json$read.doInvoke(json.clj:272) at clojure.lang.RestFn.invoke(RestFn.java:410) at clojure.lang.AFn.applyToHelper(AFn.java:161) at clojure.lang.RestFn.applyTo(RestFn.java:132) at clojure.core$apply.invoke(core.clj:619) at clojure.data.json$read_str.doInvoke(json.clj:278) at clojure.lang.RestFn.invoke(RestFn.java:410) at ridetothesun2014_strava_facade.strava$strava_json_resource.invoke(strava.clj:19)

gfredericks commented 10 years ago

Running (println body1) and (println body2) give rather unsurprising HTML-like results for me. Was your comment truncated? It looks like you were about to describe body output?

paulspencerwilliams commented 10 years ago

here goes... body1 un-truncated content..

user=> (println body1)
<!doctype html><html itemscope="" itemtype="http://schema.org/WebPage"><head><meta content="/images/google_favicon_128.png" itemprop="image"><title>Google</title><script>(function(){
window.google={kEI:"8s45U5CDK6H_ygOj4YHgBg",getEI:function(a){for(var b;a&&(!a.getAttribute||!(b=a.getAttribute("eid")));)a=a.parentNode;return b||google.kEI},https:function(){return"https:"==window.location.protocol},kEXPI:"4006,17259,4000116,4007661,4007830,4008067,4008133,4008142,4009033,4009565,4009641,4010806,4010858,4010899,4011228,4011258,4011679,4012373,4012504,4013374,4013414,4013591,4013723,4013747,4013758,4013787,4013823,4013967,4013979,4014016,4014431,4014515,4014636,4014649,4014671,4014813,4014908,4014991,4015119,4015155,4015234,4015260,4015444,4015497,4015514,4015516,4015550,4015589,4015638,4015640,4015772,4015853,4016007,4016047,4016112,4016127,4016139,4016188,4016309,4016311,4016323,4016331,4016367,4016438,4016452,4016487,4016638,8300015,8300017,8500149,8500165,8500223,10200002,10200012,10200029,10200030,10200040,10200048,10200053,10200055,10200066,10200083,10200103,10200120,10200134,10200155,10200157,10200169",kCSI:{e:"4006,17259,4000116,4007661,4007830,4008067,4008133,4008142,4009033,4009565,4009641,4010806,4010858,4010899,4011228,4011258,4011679,4012373,4012504,4013374,4013414,4013591,4013723,4013747,4013758,4013787,4013823,4013967,4013979,4014016,4014431,4014515,4014636,4014649,4014671,4014813,4014908,4014991,4015119,4015155,4015234,4015260,4015444,4015497,4015514,4015516,4015550,4015589,4015638,4015640,4015772,4015853,4016007,4016047,4016112,4016127,4016139,4016188,4016309,4016311,4016323,4016331,4016367,4016438,4016452,4016487,4016638,8300015,8300017,8500149,8500165,8500223,10200002,10200012,10200029,10200030,10200040,10200048,10200053,10200055,10200066,10200083,10200103,10200120,10200134,10200155,10200157,10200169",ei:"8s45U5CDK6H_ygOj4YHgBg"},authuser:0,ml:function(){},kHL:"en",time:function(){return(new Date).getTime()},log:function(a,b,c,h,k){var d=
new Image,f=google.lc,e=google.li,g="";d.onerror=d.onload=d.onabort=function(){delete f[e]};f[e]=d;c||-1!=b.search("&ei=")||(g="&ei="+google.getEI(h));c=c||"/"+(k||"gen_204")+"?atyp=i&ct="+a+"&cad="+b+g+"&zx="+google.time();a=/^http:/i;a.test(c)&&google.https()?(google.ml(Error("GLMM"),!1,{src:c}),delete f[e]):(d.src=c,google.li=e+1)},lc:[],li:0,y:{},x:function(a,b){google.y[a.id]=[a,b];return!1},load:function(a,b,c){google.x({id:a+l++},function(){google.load(a,b,c)})}};var l=0;})();
(function(){google.sn="webhp";google.timers={};google.startTick=function(a,b){google.timers[a]={t:{start:google.time()},bfr:!!b}};google.tick=function(a,b,g){google.timers[a]||google.startTick(a);google.timers[a].t[b]=g||google.time()};google.startTick("load",!0);
try{}catch(d){}})();
var _gjwl=location;function _gjuc(){var a=_gjwl.href.indexOf("#");if(0<=a&&(a=_gjwl.href.substring(a),0<a.indexOf("&q=")||0<=a.indexOf("#q="))&&(a=a.substring(1),-1==a.indexOf("#"))){for(var d=0;d<a.length;){var b=d;"&"==a.charAt(b)&&++b;var c=a.indexOf("&",b);-1==c&&(c=a.length);b=a.substring(b,c);if(0==b.indexOf("fp="))a=a.substring(0,d)+a.substring(c,a.length),c=d;else if("cad=h"==b)return 0;d=c}_gjwl.href="/search?"+a+"&cad=h";return 1}return 0}
function _gjh(){!_gjuc()&&window.google&&google.x&&google.x({id:"GJH"},function(){google.nav&&google.nav.gjh&&google.nav.gjh()})};
window._gjh&&_gjh();</script><style>#gbar,#guser{font-size:13px;padding-top:1px !important;}#gbar{height:22px}#guser{padding-bottom:7px !important;text-align:right}.gbh,.gbd{border-top:1px solid #c9d7f1;font-size:1px}.gbh{height:0;position:absolute;top:24px;width:100%}@media all{.gb1{height:22px;margin-right:.5em;vertical-align:top}#gbar{float:left}}a.gb1,a.gb4{text-decoration:underline !important}a.gb1,a.gb4{color:#00c !important}.gbi .gb4{color:#dd8e27 !important}.gbf .gb4{color:#900 !important}</style><style>body,td,a,p,.h{font-family:arial,sans-serif}body{margin:0;overflow-y:scroll}#gog{padding:3px 8px 0}td{line-height:.8em}.gac_m td{line-height:17px}form{margin-bottom:20px}.h{color:#36c}.q{color:#00c}.ts td{padding:0}.ts{border-collapse:collapse}em{font-weight:bold;font-style:normal}.lst{height:25px;width:496px}.gsfi,.lst{font:18px arial,sans-serif}.gsfs{font:17px arial,sans-serif}.ds{display:inline-box;display:inline-block;margin:3px 0 4px;margin-left:4px}input{font-family:inherit}a.gb1,a.gb2,a.gb3,a.gb4{color:#11c !important}body{background:#fff;color:black}a{color:#11c;text-decoration:none}a:hover,a:active{text-decoration:underline}.fl a{color:#36c}a:visited{color:#551a8b}a.gb1,a.gb4{text-decoration:underline}a.gb3:hover{text-decoration:none}#ghead a.gb2:hover{color:#fff !important}.sblc{padding-top:5px}.sblc a{display:block;margin:2px 0;margin-left:13px;font-size:11px}.lsbb{background:#eee;border:solid 1px;border-color:#ccc #999 #999 #ccc;height:30px}.lsbb{display:block}.ftl,#fll a{display:inline-block;margin:0 12px}.lsb{background:url(/images/srpr/nav_logo80.png) 0 -258px repeat-x;border:none;color:#000;cursor:pointer;height:30px;margin:0;outline:0;font:15px arial,sans-serif;vertical-align:top}.lsb:active{background:#ccc}.lst:focus{outline:none}#addlang a{padding:0 3px}</style><script></script></head><body bgcolor="#fff"><script>(function(){var src='/images/nav_logo176.png';var iesg=false;document.body.onload = function(){window.n && window.n();if (document.images){new Image().src=src;}
if (!iesg){document.f&&document.f.q.focus();document.gbqf&&document.gbqf.q.focus();}
}
})();</script><textarea id="csi" style="display:none"></textarea><div id="mngb"> <div id=gbar><nobr><b class=gb1>Search</b> <a class=gb1 href="https://www.google.co.uk/imghp?hl=en&tab=wi">Images</a> <a class=gb1 href="https://maps.google.co.uk/maps?hl=en&tab=wl">Maps</a> <a class=gb1 href="https://play.google.com/?hl=en&tab=w8">Play</a> <a class=gb1 href="https://www.youtube.com/?gl=GB&tab=w1">YouTube</a> <a class=gb1 href="https://news.google.co.uk/nwshp?hl=en&tab=wn">News</a> <a class=gb1 href="https://mail.google.com/mail/?tab=wm">Gmail</a> <a class=gb1 href="https://drive.google.com/?tab=wo">Drive</a> <a class=gb1 style="text-decoration:none" href="http://www.google.co.uk/intl/en/options/"><u>More</u> &raquo;</a></nobr></div><div id=guser width=100%><nobr><span id=gbn class=gbi></span><span id=gbf class=gbf></span><span id=gbe></span><a href="http://www.google.co.uk/history/optout?hl=en" class=gb4>Web History</a> | <a  href="/preferences?hl=en" class=gb4>Settings</a> | <a target=_top id=gb_70 href="https://accounts.google.com/ServiceLogin?hl=en&continue=https://www.google.co.uk/%3Fgfe_rd%3Dcr%26ei%3D8s45U-zMGMfR8ge7toGgCA" class=gb4>Sign in</a></nobr></div><div class=gbh style=left:0></div><div class=gbh style=right:0></div>  </div><center><br clear="all" id="lgpd"><div id="lga"><div style="padding:28px 0 3px"><div title="Google" align="left" id="hplogo" onload="window.lol&&lol()" style="height:110px;width:276px;background:url(/images/srpr/logo9w.png) no-repeat"><div nowrap="" style="color:#777;font-size:16px;font-weight:bold;position:relative;top:70px;left:218px">UK</div></div></div><br></div><form action="/search" name="f"><table cellpadding="0" cellspacing="0"><tr valign="top"><td width="25%">&nbsp;</td><td align="center" nowrap=""><input name="ie" value="ISO-8859-1" type="hidden"><input value="en-GB" name="hl" type="hidden"><input name="source" type="hidden" value="hp"><div class="ds" style="height:32px;margin:4px 0"><input style="color:#000;margin:0;padding:5px 8px 0 6px;vertical-align:top" autocomplete="off" class="lst" value="" title="Google Search" maxlength="2048" name="q" size="57"></div><br style="line-height:0"><span class="ds"><span class="lsbb"><input class="lsb" value="Google Search" name="btnG" type="submit"></span></span><span class="ds"><span class="lsbb"><input class="lsb" value="I'm Feeling Lucky" name="btnI" onclick="if(this.form.q.value)this.checked=1; else top.location='/doodles/'" type="submit"></span></span></td><td class="fl sblc" align="left" nowrap="" width="25%"><a href="/advanced_search?hl=en-GB&amp;authuser=0">Advanced search</a><a href="/language_tools?hl=en-GB&amp;authuser=0">Language tools</a></td></tr></table><input id="gbv" name="gbv" type="hidden" value="1"></form><div id="gac_scont"></div><div style="font-size:83%;min-height:3.5em"><br></div><span id="footer"><div style="font-size:10pt"><div style="margin:19px auto;text-align:center" id="fll"><a href="/intl/en/ads/">Advertising&nbsp;Programmes</a><a href="/services/">Business Solutions</a><a href="https://plus.google.com/103583604759580854844" rel="publisher">+Google</a><a href="/intl/en/about.html">About Google</a><a href="https://www.google.co.uk/setprefdomain?prefdom=US&amp;sig=0_XEJSPl-wZEtDJRGsGJpCZl6Eg9M%3D" id="fehl">Google.com</a></div></div><p style="color:#767676;font-size:8pt">&copy; 2013 - <a href="/intl/en/policies/">Privacy & Terms</a></p></span></center><div id=xjsd></div><div id=xjsi data-jiis="bp"><script>if(google.y)google.y.first=[];(function(){function b(a){window.setTimeout(function(){var c=document.createElement("script");c.src=a;document.getElementById("xjsd").appendChild(c)},0)}google.dljp=function(a){google.xjsu=a;b(a)};google.dlj=b;})();
if(!google.xjs){window._=window._||{};window._._DumpException=function(e){throw e};if(google.timers&&google.timers.load.t){google.timers.load.t.xjsls=new Date().getTime();}google.dljp('/xjs/_/js/k\x3dxjs.hp.en_US.R4DVl36W9_k.O/m\x3dsb_he,pcc/rt\x3dj/d\x3d1/sv\x3d1/rs\x3dAItRSTNqZSDsLcIHa1WZbboE5N71eAInDA');google.xjs=1;}google.pmc={"sb_he":{"agen":true,"cgen":true,"client":"heirloom-hp","dh":true,"ds":"","eqch":true,"fl":true,"host":"google.co.uk","jam":0,"jsonp":true,"msgs":{"dym":"Did you mean:","lcky":"I\u0026#39;m Feeling Lucky","lml":"Learn more","oskt":"Input tools","psrc":"This search was removed from your \u003Ca href=\"/history\"\u003EWeb History\u003C/a\u003E","psrl":"Remove","sbit":"Search by image","srch":"Google Search"},"ovr":{},"pq":"","qcpw":false,"scd":10,"sce":5,"stok":"R2FDB_tf5WfSXeEbGKpqotuLCIk"},"pcc":{}};google.y.first.push(function(){if(google.med){google.med('init');google.initHistory();google.med('history');}});if(google.j&&google.j.en&&google.j.xi){window.setTimeout(google.j.xi,0);}</script></div><script>(function(){if(google.timers&&google.timers.load.t){var b,c,d,e,g=function(a,f){a.removeEventListener?(a.removeEventListener("load",f,!1),a.removeEventListener("error",f,!1)):(a.detachEvent("onload",f),a.detachEvent("onerror",f))},h=function(a){e=(new Date).getTime();++c;a=a||window.event;a=a.target||a.srcElement;g(a,h)},k=document.getElementsByTagName("img");b=k.length;for(var l=c=0,m;l<b;++l)m=k[l],m.complete||"string"!=typeof m.src||!m.src?++c:m.addEventListener?(m.addEventListener("load",h,!1),m.addEventListener("error",
h,!1)):(m.attachEvent("onload",h),m.attachEvent("onerror",h));d=b-c;var n=function(){if(google.timers.load.t){google.timers.load.t.ol=(new Date).getTime();google.timers.load.t.iml=e;google.kCSI.imc=c;google.kCSI.imn=b;google.kCSI.imp=d;void 0!==google.stt&&(google.kCSI.stt=google.stt);google.csiReport&&google.csiReport()}};window.addEventListener?window.addEventListener("load",n,!1):window.attachEvent&&
window.attachEvent("onload",n);google.timers.load.t.prt=e=(new Date).getTime()};})();
</script></body></html>
paulspencerwilliams commented 10 years ago

As you can see, body2 has DIFFERENT html; less html with no javascript etc.

May this be the reason that my canned responses fail when played back from a cassette?

gfredericks commented 10 years ago

BTW, see this for help formatting the HTML in github comments.

gfredericks commented 10 years ago

So does (= body1 body2) return false for you?

paulspencerwilliams commented 10 years ago

Doh! I knew that, my mind was in a different context, and I just didn't think! Updated.

paulspencerwilliams commented 10 years ago
user=> (= body1 body2)
false
gfredericks commented 10 years ago

Okay, the formatting on body1 is better, but I don't think you've posted body2?

Is body2 just a truncated version of body1? I.e., does (.startsWith body1 body2) return true?

paulspencerwilliams commented 10 years ago
#'user/body2
user=> (println body2)
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML>
paulspencerwilliams commented 10 years ago

Replication steps...

Quit repl

rm -rf cass*
lein repl

Then copy and paste

(ns user
  (:require [clj-http.client :as client]
            [vcr-clj.clj-http :refer [with-cassette]]))

;; first time, records actual http call
(def body1 (:body (with-cassette "foo" (client/get "https://google.com"))))

;; second time, uses cassette
(def body2 (:body (with-cassette "foo" (client/get "https://google.com"))))

;; returning true indicates to me this is working correctly
user=> (= body1 body2)
false
gfredericks commented 10 years ago

Oh that's terribly interesting. Time to check versions of things: vcr-clj, clj-http, and clojure -- my hunch is maybe you're forcing a different version of clj-http? If that's the case we might be able to fix this by upgrading vcr-clj to use a newer version.

gfredericks commented 10 years ago

lein deps :tree will give more information about what dependencies are clobbering each other.

paulspencerwilliams commented 10 years ago
Possibly confusing dependencies found:
[ring "1.2.2"] -> [ring/ring-devel "1.2.2"] -> [clj-stacktrace "0.2.5"]
 overrides
[com.taoensso/carmine "2.4.5"] -> [com.taoensso/timbre "2.7.1"] -> [clj-stacktrace "0.2.7"]

Consider using these exclusions:
[com.taoensso/carmine "2.4.5" :exclusions [clj-stacktrace]]

 [clj-http "0.9.1"]
   [commons-io "2.4" :exclusions [[org.clojure/clojure]]]
   [crouton "0.1.1" :exclusions [[org.clojure/clojure]]]
     [org.jsoup/jsoup "1.7.1"]
   [org.apache.httpcomponents/httpclient "4.3.2" :exclusions [[org.clojure/clojure]]]
     [commons-logging "1.1.3"]
   [org.apache.httpcomponents/httpcore "4.3.2" :exclusions [[org.clojure/clojure]]]
   [org.apache.httpcomponents/httpmime "4.3.2" :exclusions [[org.clojure/clojure]]]
   [org.clojure/tools.reader "0.8.3" :exclusions [[org.clojure/clojure]]]
   [potemkin "0.3.4" :exclusions [[org.clojure/clojure]]]
     [clj-tuple "0.1.2"]
     [riddley "0.1.6"]
 [clj-time "0.6.0"]
   [joda-time "2.2"]
 [clojure-complete "0.2.3" :exclusions [[org.clojure/clojure]]]
 [com.gfredericks/vcr-clj "0.3.3"]
   [fs "1.3.3"]
     [org.apache.commons/commons-compress "1.3"]
   [org.clojure/data.codec "0.1.0"]
 [com.taoensso/carmine "2.4.5"]
   [com.taoensso/nippy "2.5.2"]
     [org.iq80.snappy/snappy "0.3"]
     [org.tukaani/xz "1.4"]
   [com.taoensso/timbre "2.7.1"]
   [commons-pool "1.6"]
 [compojure "1.1.6"]
   [clout "1.1.0"]
   [org.clojure/core.incubator "0.1.0"]
 [midje "1.6.3"]
   [colorize "0.1.1" :exclusions [[org.clojure/clojure]]]
   [commons-codec "1.9"]
   [dynapath "0.2.0"]
   [gui-diff "0.5.0"]
     [org.clojars.trptcolin/sjacket "0.1.3" :exclusions [[org.clojure/clojure]]]
       [net.cgrand/parsley "0.9.1"]
       [net.cgrand/regex "1.1.0"]
   [ordered "1.2.0" :exclusions [[org.clojure/clojure]]]
   [org.clojure/core.unify "0.5.2" :exclusions [[org.clojure/clojure]]]
   [org.clojure/math.combinatorics "0.0.7"]
   [org.clojure/tools.macro "0.1.5"]
   [org.clojure/tools.namespace "0.2.4"]
   [slingshot "0.10.3"]
   [swiss-arrows "1.0.0"]
   [utilize "0.2.3" :exclusions [[org.clojure/clojure]]]
 [org.clojure/clojure "1.5.1"]
 [org.clojure/data.json "0.2.4"]
 [org.clojure/tools.nrepl "0.2.3" :exclusions [[org.clojure/clojure]]]
 [ring/ring-json "0.3.0"]
   [cheshire "5.3.1"]
     [com.fasterxml.jackson.core/jackson-core "2.3.1"]
     [com.fasterxml.jackson.dataformat/jackson-dataformat-smile "2.3.1"]
     [tigris "0.1.1"]
 [ring "1.2.2"]
   [ring/ring-core "1.2.2"]
     [commons-fileupload "1.3"]
     [ring/ring-codec "1.0.0"]
   [ring/ring-devel "1.2.2"]
     [clj-stacktrace "0.2.5"]
     [hiccup "1.0.3"]
     [ns-tracker "0.2.2"]
       [org.clojure/java.classpath "0.2.2"]
   [ring/ring-jetty-adapter "1.2.2"]
     [org.eclipse.jetty/jetty-server "7.6.8.v20121106"]
       [org.eclipse.jetty.orbit/javax.servlet "2.5.0.v201103041518"]
       [org.eclipse.jetty/jetty-continuation "7.6.8.v20121106"]
       [org.eclipse.jetty/jetty-http "7.6.8.v20121106"]
         [org.eclipse.jetty/jetty-io "7.6.8.v20121106"]
           [org.eclipse.jetty/jetty-util "7.6.8.v20121106"]
   [ring/ring-servlet "1.2.2"]
paulspencerwilliams commented 10 years ago

Just replicated in brand new lein project with project.clj:

(defproject what "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.5.1"]
                 [com.gfredericks/vcr-clj "0.3.3"]
                 [clj-http "0.9.1"]]
)

and in repl:

^Cvagrant@precise64:~/src/what$ lein repl
nREPL server started on port 47761 on host 127.0.0.1
REPL-y 0.3.0
Clojure 1.5.1
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (ns user
  #_=>   (:require [clj-http.client :as client]
  #_=>             [vcr-clj.clj-http :refer [with-cassette]]))
nil
user=> 

user=> ;; first time, records actual http call

user=> (def body1 (:body (with-cassette "foo" (client/get "https://google.com"))))
Recording new foo cassette...
Serializing...
#'user/body1
user=> 

user=> ;; second time, uses cassette

user=> (def body2 (:body (with-cassette "foo" (client/get "https://google.com"))))
Running test with existing foo cassette...
#'user/body2
user=> 

user=> ;; returning true indicates to me this is working correctly

user=> (= body1 body2)
false
gfredericks commented 10 years ago

I just realized that vcr-clj doesn't even declare a dependency on clj-http, which is why leiningen doesn't even recognize an issue.

You're using 0.9.1, while I've done all my testing with 0.7.7.

And I confirmed that switching to 0.9.1 reproduces this.

More interestingly though, when I change the queries to go to https://www.google.com/, as the redirect message suggests, the problem goes away. So I think https was a red herring and the actual problem is redirects, which might suggest an interim workaround for you?

gfredericks commented 10 years ago

But I'll definitely work on fixing it to work with the latest clj-http release.

gfredericks commented 10 years ago

Shallow analysis makes me think that clj-http has a mechanism for following redirects that's higher up than clj-http.core/request -- if you look in the cassette file you can see it records both the original 301 response and the subsequent full response. So I think the problem is that clj-http is not following the redirect on playback, maybe because we aren't accurately enough reproducing the response object.

paulspencerwilliams commented 10 years ago

Not sure whether implicit redirects are evident, certainly I don't do anything explicitly. I can't see any 301s in the resultant cassette. I'm querying the Strava api, and will investigate tomorrow. Do you know whether many tools on OSX / Linux can sniff HTTPS traffic in sufficient detail to confirm this?

Your hypothesis definitely sounds plausible! Cheers for the help, and hope I helped a little more with a little steering ;-)

gfredericks commented 10 years ago

An easy way to tell if a URL gives a redirect is via curl:

$ curl -I google.com
HTTP/1.1 301 Moved Permanently
Location: http://www.google.com/
Content-Type: text/html; charset=UTF-8
Date: Tue, 01 Apr 2014 00:15:59 GMT
Expires: Thu, 01 May 2014 00:15:59 GMT
Cache-Control: public, max-age=2592000
Server: gws
Content-Length: 219
X-XSS-Protection: 1; mode=block
X-Frame-Options: SAMEORIGIN
Alternate-Protocol: 80:quic

It might be that there's some more fundamental problem that's manifesting itself differently for you. But I don't think it can simply be the case that https doesn't work, since https://www.google.com/ works fine for me.

Oh and of course the other short-term workaround would be to use an older version of clj-http if that's acceptable (presumably 7.7.1). I intend to dig into this more tomorrow.

gfredericks commented 10 years ago

I think the cause is that the new versions of clj-http use a custom map type for headers, which (maybe among other things) provides case-insensitivity. Reading the serialized response from the cassette just resulted in a generic clojure map, and so header lookups fail (in particular the redirect code tries to lookup the "location" header but since it's actually called "Location" it gets nil and gives up trying to follow the redirect).

gfredericks commented 10 years ago

Will have to think about how to do serialization a bit more rigorously. Ideally without defining print-method on 3rd-party types.

paulspencerwilliams commented 10 years ago

Right, I've pinned clj-http to 0.7.7 and, with only a small change to a headers map (passing string names rather than keywords), it works. However, casting my mind back to when I initially wrote this strava abstraction, I saw no 301 redirects when playing with curl so I'm not convinced redirects are THE root cause - maybe another issue..

I will try to find a better set of replication steps although in my particular case, one of the http headers is a Strava access token. I'll experiment and try to share this evening - is email the best way of sending over a TEST strava token as I won't want to publish this in the issue tracker!

gfredericks commented 10 years ago

To be clear, the discovery about the headers serialization problem means that redirects are not the root cause, but a symptom. Your problem could presumably be anything related to headers.

paulspencerwilliams commented 10 years ago

I have simple replication steps using my repo if that would help?

In bash etc,

export STRAVA_ACCESS_TOKEN={token I will email you}

If you

git@github.com:BlackPepperSoftware/runtothesun2014-strava-facade.git

Load up a

lein repl

Require the appropriate libraries

(ns user (:use ridetothesun2014-strava-facade.strava)
              (:require [clj-http.client :as client]
                             [vcr-clj.clj-http :refer [with-cassette]]))

Execute

(strava-metrics) 

a few times to provide the call is idempotent, and repeatable.

Execute

(with-cassette :foo (strava-metrics))

once which will display

Recording new :foo cassette...
Serializing...

and then execute

(with-cassette :foo (strava-metrics)) 

which will output

Running test with existing :foo cassette...

Exception JSON error (unexpected character):   clojure.data.json/-read (json.clj:226)
gfredericks commented 10 years ago

Try vcr-clj version 0.3.4-alpha1. I fixed the headers issue in a way that I don't like as a permanent solution, but wanted to make sure that you could verify that this fixes your problem.

paulspencerwilliams commented 10 years ago

Yes - that fixes the problem! So you expect to refactor out a better internal solution?

gfredericks commented 10 years ago

Right, but externally it should be basically the same, and the chances that you'll have problems with 0.3.4-alpha1 are pretty low. So you could probably just keep using that if you wanted to stop worrying about it.

paulspencerwilliams commented 10 years ago

Okay, will do and I'll keep watching this repo for future releases. Cheers for the help and being so responsive.

gfredericks commented 9 years ago

Having somebody else run into this compelled me to come up with a cleaner solution than the one I put in the alpha release, so this should now be fixed in the latest release: 0.4.1.