On 02/28/2011 11:11 AM, Bryan Call wrote:
I have seen people writing plugins pass incorrect locations to APIs.
For example you can pass a client request header to an API that wants
the url without any errors from the compiler.
Looking at the three APIs below it is hard to tell the location/offset
really point to:
tsapi TSReturnCode TSHttpTxnClientReqGet(TSHttpTxn txnp, TSMBuffer*
bufp, TSMLoc* offset);
tsapi TSReturnCode TSHttpHdrUrlGet(TSMBuffer bufp, TSMLoc offset,
TSMLoc* locp);
tsapi const char* TSUrlSchemeGet(TSMBuffer bufp, TSMLoc offset, int
*length);
In a specific case that happened last week a person was taking bufp
and offset from TSHttpTxnClientReqGet() and passing the values to
TSUrlSchemeGet(). Since it didn't give him compile errors he only saw
the problem when the server was under load and it would core dump.
Yes, this is indeed a pretty weak and confusing arrangement we have
right now, with TSMLoc being the kitchen sink :/. The one thing that
comes to mind though, as we discussed on IRC, is that separating
"request" and "response" types probably doesn't make sense, because it
could really complicate and duplicate many APIs.
Btw, and not that it's an excuse not to do this, but as of v2.1.6, the
implementation of these APIs have (by default) a much better validation
of input parameters. E.g. they will do asserts like
sdk_assert((sdk_sanity_check_mime_hdr_handle(mh_mloc) == TS_SUCCESS) ||
(sdk_sanity_check_http_hdr_handle(mh_mloc) == TS_SUCCESS));
Which means, if you pass in a TSMLoc of the "wrong" type to a particular
API, you will notice immediately. This was part of the major cleanup
efforts that we did recently to the APIs. It's quite likely there are
areas for improvements here too, but the "foundation" is there for
making the APIs safer and easier to use.
One way to fix this would be to change locations to specific types.
If we want to take it a step further the buffer and location could be
combined and given a specific type simplifying the APIs.
Yeah, I think it makes sense to "combine" the two together, as long as
there is not some significant number of APIs that only requires one or
the other. Anything to make the APIs easier is a Good Thing :).
Cheers!
-- leif
