Repository: codemanki/cloudscraper (js)

Issue:
Improve cloudflare challenge detection
Since this is based on cloudflare-scrape:

https://github.com/Anorov/cloudflare-scrape/blob/d56c981bfebdb3a53f6437be8c823888c15155d5/cfscrape/__init__.py#L55-L61
```python
    def is_cloudflare_challenge(self, resp):
        return (
            resp.status_code == 503
            and resp.headers.get("Server", "").startswith("cloudflare")
            and b"jschl_vc" in resp.content
            and b"jschl_answer" in resp.content
        )
```

However, the status code and headers aren't being checked prior to searching the response body via regex. At very least, this is a performance issue.

Pull request:
Improve cloudflare detection
Closes #76 

Patch:
diff --git a/index.js b/index.js
index 24de35f..98ef1ac 100644
--- a/index.js
+++ b/index.js
@@ -119,19 +119,36 @@ function processRequestResponse (options, error, response, body) {
   var isChallengePresent;
   var isRedirectChallengePresent;
 
+  // Encoding is null so body should be a buffer object
   if (error || !body || !body.toString) {
     // Pure request error (bad connection, wrong url, etc)
     return callback(new errors.RequestError(error, options, response));
   }
 
-  stringBody = body.toString('utf8');
+  response.isCloudflare = response.statusCode > 499 &&
+    /^cloudflare/i.test('' + response.caseless.get('server')) &&
+    /text\/html/i.test('' + response.caseless.get('content-type'));
 
-  try {
-    validate(options, response, stringBody);
-  } catch (error) {
-    return callback(error);
+  if (response.isCloudflare) {
+    if (body.length < 1) {
+      // This is a 5xx Cloudflare response with an empty body.
+      return callback(new errors.CloudflareError(response.statusCode, options, response));
+    }
+
+    stringBody = body.toString('utf8');
+
+    try {
+      validate(options, response, stringBody);
+    } catch (error) {
+      return callback(error);
+    }
+  }
+
+  if (!response.isCloudflare || response.statusCode !== 503) {
+    return processResponseBody(options, response, body);
   }
 
+  // This is a Cloudflare response with 503 status, check for challenges.
   isChallengePresent = stringBody.indexOf('a = document.getElementById(\'jschl-answer\');') !== -1;
   isRedirectChallengePresent = stringBody.indexOf('You are being redirected') !== -1 || stringBody.indexOf('sucuri_cloudproxy_js') !== -1;
   // isTargetPage = !isChallengePresent && !isRedirectChallengePresent;
@@ -284,12 +301,14 @@ function processResponseBody (options, response, body) {
     // object. This changes the response.body so it is as expected.
     response.body = body;
 
-    // In case of real encoding, try to validate the response and find
-    // potential errors there, otherwise return the response as is.
-    try {
-      validate(options, response, body);
-    } catch (error) {
-      return callback(error);
+    if (response.isCloudflare) {
+      // In case of real encoding, try to validate the response and find
+      // potential errors there, otherwise return the response as is.
+      try {
+        validate(options, response, body);
+      } catch (error) {
+        return callback(error);
+      }
     }
   }
 


Test patch:
diff --git a/test/helper.js b/test/helper.js
index eab08b6..00a5094 100644
--- a/test/helper.js
+++ b/test/helper.js
@@ -1,7 +1,8 @@
-var request = require('./rp');
-var sinon   = require('sinon');
-var fs      = require('fs');
-var path    = require('path');
+var request  = require('./rp');
+var sinon    = require('sinon');
+var fs       = require('fs');
+var path     = require('path');
+var caseless = require('caseless');
 
 var defaultParams = {
   // Since cloudscraper wraps the callback, just ensure callback is a function
@@ -34,11 +35,29 @@ module.exports = {
   },
   defaultParams: defaultParams,
   fakeResponse: function (template) {
-    return Object.assign({
+    var response = Object.assign({
       statusCode: 200,
-      headers: defaultParams.headers,
       body: ''
     }, template);
+
+    response.headers = Object.assign({}, defaultParams.headers, template.headers);
+
+    response.caseless = caseless(response.headers);
+    return response;
+  },
+  cloudflareResponse: function (template) {
+    var response = Object.assign({
+      statusCode: 503,
+      body: ''
+    }, template);
+
+    response.headers = Object.assign({}, defaultParams.headers, {
+      'server': 'cloudflare',
+      'content-type': 'text/html; charset=UTF-8'
+    }, template.headers);
+
+    response.caseless = caseless(response.headers);
+    return response;
   },
   extendParams: function (params) {
     // Extend target with the default params and provided params
@@ -49,11 +68,14 @@ module.exports = {
   },
   fakeRequest: function (template) {
     // In this context, fake is the request result
-    var fake = Object.assign({
-      error: null,
-      // Set the default fake statusCode to 500 if an error is provided
-      response: { statusCode: template.error ? 500 : 200 }
-    }, template);
+    var fake = Object.assign({ error: null }, template);
+
+    if (!('response' in fake)) {
+      fake.response = this.fakeResponse({
+        // Set the default response statusCode to 500 if an error is provided
+        statusCode: template.error ? 500 : 200
+      });
+    }
 
     // Use the body from fake response if the template doesn't provide it
     if (!('body' in fake)) {
diff --git a/test/test-errors.js b/test/test-errors.js
index cab5112..13480af 100644
--- a/test/test-errors.js
+++ b/test/test-errors.js
@@ -45,9 +45,32 @@ describe('Cloudscraper', function () {
     expect(promise).to.be.rejectedWith(errors.RequestError).and.notify(done);
   });
 
+  it('should return error if cloudflare response is empty', function (done) {
+    var expectedResponse = helper.cloudflareResponse({
+      statusCode: 504,
+      body: Buffer.from('')
+    });
+
+    Request.callsFake(helper.fakeRequest({ response: expectedResponse }));
+
+    var promise = cloudscraper.get(uri, function (error) {
+      // errorType 1, means captcha is served
+      expect(error).to.be.instanceOf(errors.CloudflareError);
+      expect(error).to.have.property('error', 504);
+      expect(error).to.have.property('errorType', 2);
+      expect(error.message).to.be.equal('504, Gateway Timeout');
+
+      expect(Request).to.be.calledOnceWithExactly(helper.defaultParams);
+
+      expect(error.response).to.be.equal(expectedResponse);
+      expect(error.response.body).to.be.equal(expectedResponse.body);
+    });
+
+    expect(promise).to.be.rejectedWith(errors.CloudflareError).and.notify(done);
+  });
+
   it('should return error if captcha is served by cloudflare', function (done) {
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('captcha.html')
     });
 
@@ -73,7 +96,7 @@ describe('Cloudscraper', function () {
     // Error codes: 1012, 1011, 1002, 1000, 1004, 1010, 1006, 1007, 1008
     // Error codes can also be the same as the HTTP status code in the 5xx range.
 
-    var expectedResponse = helper.fakeResponse({
+    var expectedResponse = helper.cloudflareResponse({
       statusCode: 500,
       body: helper.getFixture('access_denied.html')
     });
@@ -101,7 +124,7 @@ describe('Cloudscraper', function () {
     // Error codes: 1012, 1011, 1002, 1000, 1004, 1010, 1006, 1007, 1008
     // Error codes can also be the same as the HTTP status code in the 5xx range.
 
-    var expectedResponse = helper.fakeResponse({
+    var expectedResponse = helper.cloudflareResponse({
       statusCode: 500,
       body: helper.getFixture('access_denied.html').replace('1006', '504')
     });
@@ -129,7 +152,7 @@ describe('Cloudscraper', function () {
     // Error codes: 1012, 1011, 1002, 1000, 1004, 1010, 1006, 1007, 1008
     // Error codes can also be the same as the HTTP status code in the 5xx range.
 
-    var expectedResponse = helper.fakeResponse({
+    var expectedResponse = helper.cloudflareResponse({
       statusCode: 500,
       body: helper.getFixture('access_denied.html').replace('1006', '5111')
     });
@@ -165,8 +188,7 @@ describe('Cloudscraper', function () {
     });
 
     // Cloudflare is enabled for site. It returns a page with js challenge
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_09_06_2016.html')
     });
 
@@ -199,7 +221,7 @@ describe('Cloudscraper', function () {
 
   it('should return error if body is undefined', function (done) {
     Request.callsFake(helper.fakeRequest({
-      response: { statusCode: 500 }
+      response: helper.cloudflareResponse({ body: undefined })
     }));
 
     var promise = cloudscraper.get(uri, function (error) {
@@ -207,16 +229,17 @@ describe('Cloudscraper', function () {
       expect(error).to.have.property('error', null);
       expect(error).to.have.property('errorType', 0);
 
-      expect(Request).to.be.calledOnceWithExactly(helper.defaultParams);
+      assert.equal(error.response.statusCode, 503, 'status code');
 
       expect(error.response.body).to.be.equal(undefined);
+      expect(Request).to.be.calledOnceWithExactly(helper.defaultParams);
     });
 
     expect(promise).to.be.rejectedWith(errors.RequestError).and.notify(done);
   });
 
   it('should return error if challenge page failed to be parsed', function (done) {
-    var expectedResponse = helper.fakeResponse({
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('invalid_js_challenge.html')
     });
 
@@ -239,8 +262,7 @@ describe('Cloudscraper', function () {
   });
 
   it('should return error if js challenge has error during evaluation', function (done) {
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_1.html')
     });
 
@@ -267,8 +289,7 @@ describe('Cloudscraper', function () {
   });
 
   it('should return error if pass extraction fails', function (done) {
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_1.html')
     });
 
@@ -293,8 +314,7 @@ describe('Cloudscraper', function () {
   });
 
   it('should return error if challengeId extraction fails', function (done) {
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_1.html')
     });
 
@@ -319,8 +339,7 @@ describe('Cloudscraper', function () {
   });
 
   it('should return error if it was thrown by request when solving challenge', function (done) {
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_21_05_2015.html')
     });
 
@@ -353,8 +372,7 @@ describe('Cloudscraper', function () {
 
   it('should properly handle a case when after a challenge another one is returned', function (done) {
     // Cloudflare is enabled for site. It returns a page with js challenge
-    var firstResponse = helper.fakeResponse({
-      statusCode: 503,
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_09_06_2016.html')
     });
 
@@ -373,7 +391,7 @@ describe('Cloudscraper', function () {
       qs: sinon.match.object
     });
 
-    var secondResponse = helper.fakeResponse({
+    var secondResponse = helper.cloudflareResponse({
       body: helper.getFixture('captcha.html')
     });
 
@@ -402,8 +420,7 @@ describe('Cloudscraper', function () {
   it('should return error if challenge page cookie extraction fails', function (done) {
     // Cloudflare is enabled for site.
     // It returns a redirecting page if a (session) cookie is unset.
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       // The cookie extraction codes looks for the `S` variable assignment
       body: helper.getFixture('js_challenge_cookie.html').replace(/S=/gm, 'Z=')
     });
@@ -458,8 +475,7 @@ describe('Cloudscraper', function () {
       realEncoding: 'fake-encoding'
     });
 
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: {
         toString: function (encoding) {
           if (encoding === 'fake-encoding') {
@@ -497,8 +513,7 @@ describe('Cloudscraper', function () {
     var html = helper.getFixture('js_challenge_cookie.html');
     var b64 = Buffer.from('throw new Error(\'vm eval failed\');').toString('base64');
 
-    var expectedResponse = helper.fakeResponse({
-      statusCode: 503,
+    var expectedResponse = helper.cloudflareResponse({
       body: html.replace(/S='([^']+)'/, 'S=\'' + b64 + '\'')
     });
 
diff --git a/test/test-index.js b/test/test-index.js
index fee6a1e..5608072 100644
--- a/test/test-index.js
+++ b/test/test-index.js
@@ -71,8 +71,7 @@ describe('Cloudscraper', function () {
 
   it('should resolve challenge (version as on 21.05.2015) and then return page', function (done) {
     // Cloudflare is enabled for site. It returns a page with js challenge
-    var firstResponse = helper.fakeResponse({
-      statusCode: 503,
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_21_05_2015.html')
     });
 
@@ -119,8 +118,7 @@ describe('Cloudscraper', function () {
 
   it('should resolve challenge (version as on 09.06.2016) and then return page', function (done) {
     // Cloudflare is enabled for site. It returns a page with js challenge
-    var firstResponse = helper.fakeResponse({
-      statusCode: 503,
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_09_06_2016.html')
     });
 
@@ -167,8 +165,7 @@ describe('Cloudscraper', function () {
   it('should resolve 2 consequent challenges', function (done) {
     var firstParams = helper.extendParams({ resolveWithFullResponse: true });
     // First call and CF returns a challenge
-    var firstResponse = helper.fakeResponse({
-      statusCode: 503,
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_1.html')
     });
 
@@ -191,8 +188,7 @@ describe('Cloudscraper', function () {
     });
 
     // We submit a solution to the first challenge, but CF decided to give us a second one
-    var secondResponse = helper.fakeResponse({
-      statusCode: 503,
+    var secondResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_2.html')
     });
 
@@ -308,7 +304,7 @@ describe('Cloudscraper', function () {
   });
 
   it('should set the given cookie and then return page', function (done) {
-    var firstResponse = helper.fakeResponse({
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_cookie.html')
     });
 
@@ -353,8 +349,7 @@ describe('Cloudscraper', function () {
       proxy: 'https://example-proxy-site.dev/path/'
     });
 
-    var firstResponse = helper.fakeResponse({
-      statusCode: 503,
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_03_12_2018_1.html')
     });
 
@@ -404,7 +399,7 @@ describe('Cloudscraper', function () {
 
     var firstParams = helper.extendParams({ jar: customJar });
 
-    var firstResponse = helper.fakeResponse({
+    var firstResponse = helper.cloudflareResponse({
       body: helper.getFixture('js_challenge_cookie.html')
     });