On Nov 12, 2013, at 3:27 PM, [email protected] wrote:
> TS-2316: header_rewrite: fixed segfaults in ConditionPath::append_value()
>
> -void
> -ConditionPath::append_value(std::string& s, const Resources& res)
> -{
> - int path_len = 0;
> - const char *path = TSUrlPathGet(res._rri->requestBufp,
> res._rri->requestUrl, &path_len);
> - TSDebug(PLUGIN_NAME, "Appending PATH to evaluation value: %.*s", path_len,
> path);
> - s.append(path, path_len);
> +void ConditionPath::append_value(std::string& s, const Resources& res) {
Sad panda; why did you undo the consistent changes from a previous patch? The {
should be on its own line, no ?
> + TSMBuffer bufp;
> + TSMLoc url_loc;
> +
> + if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) {
> + int path_length;
> + const char *path = TSUrlPathGet(bufp, url_loc, &path_length);
Also, I’m not sure why this change fixes a segfault? I.e. why the change from
using the rri->requestUrl to use TSHttpTxnPristineUrlGet ? Is that to make it
work with both remap plugins and global hooks? If so, the commit comment is
feather misleading.
What I’d probably prefer to see is to use the “rri” when possible, and other
APIs when not. The “rri” structure is only populated in remap plugins (of
course), but we should use it for efficiency when available (IMO at least).
— Leif
