Merge pull request request#767 from stash/stash/sync-cookie-api

Use tough-cookie CookieJar sync API

README.md

@@ -352,14 +352,13 @@ request('http://www.google.com', function () {
   request('http://images.google.com')
 })
 ```
-OR
 
-Note that `setCookie` requires at least three parameters, and the last is required to be a callback.
+OR
 
 ```javascript
 var j = request.jar()
 var cookie = request.cookie('your_cookie_here')
-j.setCookie(cookie, uri, function (err, cookie){})
+j.setCookie(cookie, uri);
 request({url: 'http://www.google.com', jar: j}, function () {
   request('http://images.google.com')
 })

index.js

@@ -12,12 +12,7 @@
 //    See the License for the specific language governing permissions and
 //    limitations under the License.
 
-var optional = require('./lib/optional')
-  , cookie = optional('tough-cookie')
-  , Cookie = cookie && cookie.Cookie
-  , CookieJar = cookie && cookie.CookieJar
-  , cookieJar = CookieJar && new CookieJar
-
+var cookies = require('./lib/cookies')
   , copy = require('./lib/copy')
   , Request = require('./request')
   ;
@@ -148,10 +143,8 @@ request.del = function (uri, options, callback) {
   return requester(params)(params.uri || null, params.options, params.callback)
 }
 request.jar = function () {
-  return new CookieJar
+  return cookies.jar();
 }
 request.cookie = function (str) {
-  if (str && str.uri) str = str.uri
-  if (typeof str !== 'string') throw new Error("The cookie function only accepts STRING as param")
-  return Cookie.parse(str)
+  return cookies.parse(str);
 }

lib/cookies.js

@@ -0,0 +1,36 @@
+var optional = require('./optional')
+  , tough = optional('tough-cookie')
+  , Cookie = tough && tough.Cookie
+  , CookieJar = tough && tough.CookieJar
+  ;
+
+exports.parse = function(str) {
+  if (str && str.uri) str = str.uri
+  if (typeof str !== 'string') throw new Error("The cookie function only accepts STRING as param")
+  if (!Cookie) {
+    return null;
+  }
+  return Cookie.parse(str)
+};
+
+// Adapt the sometimes-Async api of tough.CookieJar to our requirements
+function RequestJar() {
+  this._jar = new CookieJar();
+}
+RequestJar.prototype.setCookie = function(cookieOrStr, uri) {
+  return this._jar.setCookieSync(cookieOrStr, uri);
+};
+RequestJar.prototype.getCookieString = function(uri) {
+  return this._jar.getCookieStringSync(uri);
+};
+
+exports.jar = function() {
+  if (!CookieJar) {
+    // tough-cookie not loaded, return a stub object:
+    return {
+      setCookie: function(){},
+      getCookieString: function(){}
+    };
+  }
+  return new RequestJar();
+};

package.json

@@ -28,7 +28,7 @@
     "mime": "~1.2.9"
   },
   "optionalDependencies": {
-    "tough-cookie": "~0.10.0",
+    "tough-cookie": ">=0.12.0",
     "form-data": "~0.1.0",
     "tunnel-agent": "~0.3.0",
     "http-signature": "~0.10.0",

request.js

@@ -21,9 +21,8 @@ var optional = require('./lib/optional')
   , ForeverAgent = require('forever-agent')
   , FormData = optional('form-data')
 
-  , Cookie = optional('tough-cookie')
-  , CookieJar = Cookie && Cookie.CookieJar
-  , cookieJar = CookieJar && new CookieJar
+  , cookies = require('./lib/cookies')
+  , globalCookieJar = cookies.jar()
 
   , copy = require('./lib/copy')
   , debug = require('./lib/debug')
@@ -279,7 +278,7 @@ Request.prototype.init = function (options) {
   if (options.auth) {
     if (Object.prototype.hasOwnProperty.call(options.auth, 'username')) options.auth.user = options.auth.username
     if (Object.prototype.hasOwnProperty.call(options.auth, 'password')) options.auth.pass = options.auth.password
-    
+
     self.auth(
       options.auth.user,
       options.auth.pass,
@@ -651,18 +650,14 @@ Request.prototype.onResponse = function (response) {
     self.timeoutTimer = null
   }
 
+  var targetCookieJar = (self._jar && self._jar.setCookie)?self._jar:globalCookieJar;
   var addCookie = function (cookie) {
-    if (self._jar){
-      var targetCookieJar = self._jar.setCookie?self._jar:cookieJar;
-
-      //set the cookie if it's domain in the href's domain.
-      targetCookieJar.setCookie(cookie, self.uri.href, function(err){
-        if (err){
-            console.warn('set cookie failed,'+ err)
-        }
-      })
+    //set the cookie if it's domain in the href's domain.
+    try {
+      targetCookieJar.setCookie(cookie, self.uri.href);
+    } catch (e) {
+      self.emit('error', e);
     }
-
   }
 
   if (hasHeader('set-cookie', response.headers) && (!self._disableCookies)) {
@@ -1182,18 +1177,12 @@ Request.prototype.jar = function (jar) {
     cookies = false
     this._disableCookies = true
   } else {
-    var targetCookieJar = (jar && jar.getCookieString)?jar:cookieJar;
+    var targetCookieJar = (jar && jar.getCookieString)?jar:globalCookieJar;
     var urihref = this.uri.href
-
     //fetch cookie in the Specified host
-    targetCookieJar.getCookieString(urihref, function(err, hrefCookie){
-      if (err){
-        console.warn('get cookieString failed,' +err)
-      } else {
-        cookies = hrefCookie
-      }
-    })
-
+    if (targetCookieJar) {
+      cookies = targetCookieJar.getCookieString(urihref);
+    }
   }
 
   //if need cookie and cookie is not empty

tests/test-headers.js

@@ -9,8 +9,6 @@ try {
 var server = require('./server')
   , assert = require('assert')
   , request = require('../index')
-  , Cookie = require('tough-cookie')
-  , Jar = Cookie.CookieJar
   , s = server.createServer()
 
 s.listen(s.port, function () {
@@ -47,8 +45,8 @@ s.listen(s.port, function () {
 
   // Issue #125: headers.cookie + cookie jar
   //using new cookie module
-  var jar = new Jar()
-  jar.setCookie('quux=baz', serverUri, function(){});
+  var jar = request.jar()
+  jar.setCookie('quux=baz', serverUri);
   createTest({jar: jar, headers: {cookie: 'foo=bar'}}, function (req, res) {
     assert.ok(req.headers.cookie)
     assert.equal(req.headers.cookie, 'foo=bar; quux=baz')

tests/test-redirect.js

@@ -9,8 +9,6 @@ try {
 var server = require('./server')
   , assert = require('assert')
   , request = require('../index')
-  , Cookie = require('tough-cookie')
-  , Jar = Cookie.CookieJar
   ;
 
 var s = server.createServer()
@@ -53,8 +51,8 @@ s.listen(s.port, function () {
   }
 
   // Permanent bounce
-  var jar = new Jar()
-  jar.setCookie('quux=baz', server, function(){})
+  var jar = request.jar()
+  jar.setCookie('quux=baz', server);
   request({uri: server+'/perm', jar: jar, headers: {cookie: 'foo=bar'}}, function (er, res, body) {
     if (er) throw er
     if (res.statusCode !== 200) throw new Error('Status is not 200: '+res.statusCode)