Squid v3.3 asserts after newRequest()

Asked by Bartłomiej Leszak on 2015-12-14

Hi,

I developed an adapter which is an interface between squid and external software (library). This library outputs plain text headers to inject, so instead of using clone(), I inject these headers using newRequest() or newResponse() methods and then parse() plain text header into header object. The problem here is that newResponse() works well, while newRequest() makes squid bail out with messages:
2015/12/13 01:16:40.460| client_side.cc(2317) parseHttpRequest: HTTP Client REQUEST:
---------
GET http://www.hidden.com/cats HTTP/1.1
User-Agent: Wget/1.15 (linux-gnu)
Accept: */*
Host: www.hidden.com
Connection: Close
Proxy-Connection: Keep-Alive

----------
2015/12/13 01:16:40.464| client_side_request.cc(786) clientAccessCheckDone: The request GET http://www.hidden.com/cats is ALLOWED, because it matched 'all'
2015/12/13 01:16:40.464| assertion failed: client_side_reply.cc:693: "r->clientConnectionManager == http->getConn()"

The example adapter code (simplified for the sake of demonstration):

void Adapter::Xaction::start() {
        Must(hostx);

        const libecap::Header *h;

        h = &(hostx->virgin().header());

        libecap::Area img = h->image();

        libecap::shared_ptr<libecap::Message> adapted = libecap::MyHost().newRequest();
        Must(adapted != 0);

        adapted->header().parse(img);

        hostx->useAdapted(adapted);
}

As I said, pretty much the same piece of code is ok, when used after newResponse()... Should I do something specific to make the above work? My first guess is that this is squid's bug, so I'm currently compiling newer version (3.4.14, I don't want to mess to much with dependencies, including libecap in squid 3.5).
The host I'm testing this on is: Ubuntu 14.04 LTS x86_64, libecap 0.2.0, squid 3.3.8.

Best Regards
BL

Question information

Language:
English Edit question
Status:
Solved
For:
eCAP Edit question
Assignee:
No assignee Edit question
Solved by:
Bartłomiej Leszak
Solved:
2015-12-20
Last query:
2015-12-20
Last reply:
2015-12-15
Bartłomiej Leszak (bartekl) said : #1

I've just checked squid 3.4.14 - same issue, exactly the same message.

Alex Rousskov (rousskov) said : #2

This looks like a Squid eCAP implementation bug to me. If you are sure it exists in Squid v3.5 or above (libecap v1.0), then you should report it to the Squid project. If it does not exist in the supported versions, you should consider an upgrade.

Meanwhile or if you cannot upgrade, consider cloning the message and replacing the headers as needed instead.

Bartłomiej Leszak (bartekl) said : #3

Thanks for your answer, I checked squid 3.5.12 and libecap 1.0.0 on Fedora 23 x86_64 and there is still the same issue:

2015/12/16 01:44:18 kid1| assertion failed: client_side_reply.cc:687: "r->clientConnectionManager == http->getConn()"

So, it seems that I need to do both: file a bug (I'll do it once I have more time) and rewrite my adapter...

Alex Rousskov (rousskov) said : #4

Yes, you may have to do both. Whether you need to rewrite the adapter probably depends on whether you can get Squid developers confirm and fix the bug quickly enough, but filing the bug report is the right first step. If you can share a _simple_ adapter that crashes Squid, I recommend doing that in the Squid bug report.

http://wiki.squid-cache.org/SquidFaq/AboutSquid#How_to_add_a_new_Squid_feature.2C_enhance.2C_of_fix_something.3F

Bartłomiej Leszak (bartekl) said : #5

Thanks, I'll do it once I'm done with my adapter - hopefully this weekend. More interestingly, newResponse() in respmod also fails but without an assert message, without an error message at all. Header is delivered to client, but body doesn't get delivered at all (client times out while waiting). Adapter is the same as in my first message here, just newRequest() replaced with newResponse().

As of the adapter I walked around this issue by using clone() and parse() and implementing NamedValueVisitor for cloned header to remove all headers from it later on, as parse() replaces status/request line and appends headers to the end of the header. I insist on doing it this way due to the interface to an external library, which I have no influence on.

I'll post squid bug links here for reference.

Alex Rousskov (rousskov) said : #6

If the brand new (i.e., not cloned) message should include a body, the adapter must signal that fact by calling the addBody() method of the message. Calling that method is the only way to let the host application know that the message has a body and that the host application should call abMake() and related methods to receive that body.

Bartłomiej Leszak (bartekl) said : #7

Ehh... Yes, I know. In fact I tested it with addBody() and vbMake() in last iterations (but described earlier approach here), long story short: I modified sample adapter_modifying for newRequest() tests removing all configuration of the adapter, but leaving all the code using it (which was fine for header-only tests). Once modified for newResponse() it was failing in abContent using "victim" variable... So, after all it's all fine with newResponse().

My bad, sorry for confusion, I just wanted to check the other side for the similar bug, since I'm here...

Squid bug link:
http://bugs.squid-cache.org/show_bug.cgi?id=4400

PS I guess there will be less to nothing more to do here, so I should close this Q?

Alex Rousskov (rousskov) said : #8

Thank you for confirming that newResponse() works.

Yes, I agree that this question should be closed. It is up to the Squid Project to fix the bug you have reported at
 http://bugs.squid-cache.org/show_bug.cgi?id=4400

Bartłomiej Leszak (bartekl) said : #9

Thanks for help.