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

Async callback? #2

Open
hongkongkiwi opened this issue Feb 3, 2016 · 8 comments
Open

Async callback? #2

hongkongkiwi opened this issue Feb 3, 2016 · 8 comments
Labels

Comments

@hongkongkiwi
Copy link

First of all, thank you so much for making this!! It's really fantastic and you've totally taken the pain out of writing an icap server.

I'm facing a bit of an issue and it might seem simple to you so any help is appreciated.

Essentially I want to scan incoming requests for the uri (done this), then use that uri to do a blacklist lookup or scanning from a db. The db callback is asynchronous so that means I don't know when it's going to come back.

How could I integrate this into your model? It seems that processing icap requests is synchronous. Essentially I am happy to keep receiving data, but at some point I want come back and say yes finally this is allowed, or finally this is not allowed.

Any quick examples?

@horpto
Copy link
Owner

horpto commented Feb 3, 2016

@hongkongkiwi

It's really fantastic and you've totally taken the pain out of writing an icap server.

not me, it's fork of other unmaintained repo.I'm just support and fix some bugs add add features.
(I has a project that depends on this)

The db callback is asynchronous so that means I don't know when it's going to come back.

you are right and not at the same time:) If you want just operate over uri or|and headers and not affect on http content you can really simple use async function.

On the other hand, scanning http content are really synchronous:( Thanks for this issues, I'll add callback only for this case(scanning all http content).

Essentially I want to scan incoming requests for the uri (done this), then use that uri to do a blacklist lookup or scanning from a db.

Keep calm and use async functions:)
You can just write something like (not real code):

server.request(whitelist, analyzeRequest);

function allowRequest(icapReq, icapRes, req, res) {
    // see examples/example.js, acceptRequest 
    // ..
}
function disallowrequest(icapReq, icapRes, req, res) {
    // see examples/example.js, rejectRequest 
    // ..
}

function analyzeRequest(icapReq, icapRes, req, res) {
    makeDbQuery(req.uri, req.headers, (err, answer) => {
       if (err) {
            return allowRequest();
       }
       if (answer) {
            return allowRequest();
       } else {
            return disallowRequest();
       }
    })
}

Just try test to scan with async functions and if you have a troubles, please, tell

@hongkongkiwi
Copy link
Author

I am having some trouble with multiple middleware, in theory this should work:

var testCheck = function(icapReq, icapRes, httpRequest, httpResponse, next) {
  return next();
};

server.request('*', testCheck, acceptRequest);
server.response('*', acceptRequest);

And when testing I get this in my logs:

info: [49333::server] REQMOD -    - GET http://httpbin.org/get -   
info: [49333::server] OPTIONS - ICAP/1.0 200 OK  

But after this, the request gets 'stuck'. I put a log statement in acceptRequest and it seems it never gets called. Any idea what the issue could be?

It has the same problem with:

  return acceptRequest;

I am using node v4.2.6

@hongkongkiwi
Copy link
Author

Figured it out....

The middleware is a bit different to express (where you can chain by simply passing multiple function inputs). Here you need one line per middleware. So the example becomes:

var testCheck = function(icapReq, icapRes, httpRequest, httpResponse, next) {
  if (httpRequest.uri === 'http://www.test.com') {
    return acceptRequest(icapReq, icapRes, httpRequest, httpResponse);
  }
};

server.request('*', testCheck);
server.response('*', rejectRequest);

Works well :-)

@hongkongkiwi
Copy link
Author

Hi there,

Sorry to bother you, but I have another question.... I'm attempting to just send a simple error page back on any rejection responses.

My idea is to stop processing immediately (don't even request more preview), because I would like to return the response as fast as possible.

It doesn't seem to be working. It never actually returns the data back to the client. Any idea of the issue?

Here is my code:

var rejectResponse = function(icapReq, icapRes, req, res) {
    icapRes.setIcapStatusCode(200);
    icapRes.setIcapHeaders(icapRes.headers);
    icapRes.setHttpHeaders(res.headers);
    icapRes.setHttpStatus(200);
    icapRes.send(errorPage);
};

server.request('*', acceptRequest);
server.response('*', rejectResponse);

@hongkongkiwi hongkongkiwi reopened this Feb 4, 2016
@horpto
Copy link
Owner

horpto commented Feb 4, 2016

@hongkongkiwi
please, add icapRes.writeHeaders(true);, (true - there is body, false - nope).

@hongkongkiwi
Copy link
Author

It's unfortunately not working.... Here is my full script. When I use this, it doesn't come back with the response :-(

// Setup the server and our options
var ICAPServer = require('nodecap2').ICAPServer;
var server = new ICAPServer({
  debug: true,
});
server.listen(function(port) {
  console.log('ICAP server listening on port ' + port);
});

// Setup the routes for the icap server (this is unmodified from the example)
server.options('/request', function(icapReq, icapRes, next) {
  icapRes.setIcapStatusCode(200);
  icapRes.setIcapHeaders({
    'Methods': 'REQMOD',
    'Preview': '128'
  });
  icapRes.writeHeaders(false);
  icapRes.end();
});

//  RESPMOD
server.options('/response', function(icapReq, icapRes, next) {
  icapRes.setIcapStatusCode(200);
  icapRes.setIcapHeaders({
    'Methods': 'RESPMOD',
    'Preview': '128',
    'Transfer-Preview': '*',
    'Transfer-Ignore': 'jpg,jpeg,gif,png,svg,mov,avi,mp4',
    'Transfer-Complete': '',
    'Max-Connections': '100'
  });
  icapRes.writeHeaders(false);
  icapRes.end();
});

//  return error if options path not recognized
server.options('*', function(icapReq, icapRes, next) {
  if (!icapRes.done) {
    icapRes.setIcapStatusCode(404);
    icapRes.writeHeaders(false);
    icapRes.end();
    return;
  }
  next();
});

// Our errorpage html, very simple.
var errorPage = '<html><body><h1>This page is rejected</h2></body></html>';

// This is just the same as the example
var acceptRequest = function(icapReq, icapRes, req, res, next) {
  if (!icapRes.hasFilter() && icapReq.hasPreview()) {
    icapRes.allowUnchanged();
    return;
  }
  icapRes.setIcapStatusCode(200);
  icapRes.setIcapHeaders(icapReq.headers);
  if (icapReq.isReqMod()) {
    icapRes.setHttpMethod(req);
    icapRes.setHttpHeaders(req.headers);
  } else {
    icapRes.setHttpMethod(res);
    icapRes.setHttpHeaders(res.headers);
  }
  var hasBody = icapReq.hasBody();
  if (hasBody) {
    icapRes.continuePreview();
  }
  icapRes.writeHeaders(hasBody);
  icapReq.pipe(icapRes);
};

// This is the modified one from the example, the goal is simply to send the errorPage instead of the real page.
var rejectResponse = function(icapReq, icapRes, req, res) {
    icapRes.setIcapStatusCode(200);
    icapRes.setIcapHeaders(icapRes.headers);
    // In this case force a response of text/html
    icapRes.setHttpHeaders({'Content-Type': 'text/html; charset=utf-8'});
    // User thinks the page loaded ok
    icapRes.setHttpStatus(200);
    // Since we are setting our own headers then headers is true
    icapRes.writeHeaders(true);
    // Send our fake errorPage content
    icapRes.send(errorPage);
};

// This just checks if the URL matches, in this case I want to only do URL checking and return as fast as I can, so no need to get the content preview or any additional data from the proxy.
var filterResponse = function(icapReq, icapRes, httpRequest, httpResponse, next) {
  if (parseInt(httpResponse.code) !== 200) {
    return next();
  }
  var parsed = url.parse(httpRequest.uri, true);
  if (parsed.host === 'www.vanamco.com') {
    return rejectResponse(icapReq, icapRes, httpRequest, httpResponse);
  }
  return next();
};

// Accept all requests
server.request('*', acceptRequest);
// First check all responses if it is from our magic url
server.response('*', filterResponse);
// Accept everything that doesn't match our magic url
server.response('*', acceptRequest);

Here is my test script, it's very simple. And here is my squid.conf example. I think it's straightforward but just in case.

Unfortunately it doesn't work, it simply doesn't give an answer back to the test script. Squid thinks it's having an error, so it's not returning something correctly.

@horpto
Copy link
Owner

horpto commented Feb 5, 2016

@hongkongkiwi

First, please add var url = require('url');
Second, please change line ' icapRes.setHttpMethod(res);' to ' icapRes.setHttpStatus(res);' in acceptRequest. Really docs are wrong. You should write for reqmod

icapRes.setHttpMethod(req);
icapRes.setHttpHeaders(req.headers);

and for respmod

icapRes.setHttpStatus(req.code); // but  icapRes.setHttpStatus(req); also possible
icapRes.setHttpHeaders(req.headers);

Other, do you really need respmod? You wrote that you are checking only url, maybe reqmod is better solution to check url?

I create a gitter channel. Please, let's talk there in.

@horpto
Copy link
Owner

horpto commented Feb 5, 2016

And add icapRes.end()at the end of rejectResponse.

@horpto horpto added the question label Feb 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants