You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Walt Karas <wk...@oath.com.INVALID> on 2018/01/29 18:02:13 UTC
Proposed new ts api calls
const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
These would return, from the transaction state object,
url_map.getFromURL() and url_map.getToURL() respectively, in strong
format. A null pointer would be returned if the values were empty.
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
This is intended primarily for use by XDebug and operationally it would be
infeasible to add that plugin to every remap rule in order to access those
values.
Re: Proposed new ts api calls
Posted by Leif Hedstrom <zw...@apache.org>.
> On Jan 31, 2018, at 4:33 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
>
>
>> On Jan 29, 2018, at 11:02 AM, Walt Karas <wk...@oath.com.INVALID> wrote:
>>
>> const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>>
>> const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>>
>> These would return, from the transaction state object,
>> url_map.getFromURL() and url_map.getToURL() respectively, in strong
>> format. A null pointer would be returned if the values were empty.
>
>
> Why is this needed?
>
> In a remap plugin, you have the to and from URLs already, so this would not be needed at all (just call TSUrlStringGet() on those parameters). In a regular plugin (continuation), I think we don’t have an API to get the To and From URLs, but if so, it feels like what we really need is something like
>
> rule = TSHttpRemapRuleGet(txnp);
Ok, my slight bad, I see that your new proposal is more inline with what I saw in the first emails.
— Leif
>
> and from there
>
> toURLString = TSUrlStringGet(rule->toUrl);
>
>
> Or some such. Not sure if we’d expose a struct like this, or make it an opaque handler and have getters on it, that is implementation details.
>
> — Leif
>
Re: Proposed new ts api calls
Posted by Leif Hedstrom <zw...@apache.org>.
> On Jan 29, 2018, at 11:02 AM, Walt Karas <wk...@oath.com.INVALID> wrote:
>
> const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>
> const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>
> These would return, from the transaction state object,
> url_map.getFromURL() and url_map.getToURL() respectively, in strong
> format. A null pointer would be returned if the values were empty.
Why is this needed?
In a remap plugin, you have the to and from URLs already, so this would not be needed at all (just call TSUrlStringGet() on those parameters). In a regular plugin (continuation), I think we don’t have an API to get the To and From URLs, but if so, it feels like what we really need is something like
rule = TSHttpRemapRuleGet(txnp);
and from there
toURLString = TSUrlStringGet(rule->toUrl);
Or some such. Not sure if we’d expose a struct like this, or make it an opaque handler and have getters on it, that is implementation details.
— Leif
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
Sorry that's a typo, it should not be there.
On Mon, Jan 29, 2018 at 3:53 PM, Shu Kit Chan <ch...@gmail.com> wrote:
> What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
>
> On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid> wrote:
>> const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>>
>> const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>>
>> These would return, from the transaction state object,
>> url_map.getFromURL() and url_map.getToURL() respectively, in strong
>> format. A null pointer would be returned if the values were empty.
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
Sorry you lost me. The revision superceeds the prior mail I sent, if
that's what you mean by "API write up". I tried to change it to
follow the pattern of TSUrlStringGet(), which I think does not
nul-terminate.
On Tue, Jan 30, 2018 at 12:47 PM, Alan Carroll
<so...@oath.com.invalid> wrote:
> May I presume your API write up, not your previous reply, is correct with
> regard to null termination of the string?
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
May I presume your API write up, not your previous reply, is correct with
regard to null termination of the string?
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
Looks mostly good to me, except a minor coding issue. The API documentation
is on that PR as well, anyone who is concerned should look at it. I think
it avoids the issues that have been brought up on this exchange.
On Tue, Feb 20, 2018 at 5:52 PM, Walt Karas <wk...@oath.com.invalid> wrote:
> I've issued a pull request for the implementation of this:
>
> https://github.com/apache/trafficserver/pull/3145
>
> On Thu, Feb 1, 2018 at 12:49 PM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> >
> >> On Feb 1, 2018, at 9:35 AM, Alan Carroll <so...@oath.com.INVALID>
> wrote:
> >>
> >> That's a good suggestion, although shouldn't it be
> >>
> >> TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc*
> obj);
> >>
> >> That is the TSMLoc should be an out parameter, not an in. In that case
> I'd
> >> drop the ...StringGet() and use the generic TSUrlStringGet() if a
> string is
> >> needed.
> >
> >
> > I’m definitely not convinced we need those additional StringGet() APIs,
> when the existing one would definitely work. As for the unused marshaling
> buffer, we can look into that for 8.0.0, and decide if it’s never needed,
> and clean that up and fix the APIs accordingly. I’d much rather prefer
> that, that introduce new, duplicated functionality APIs.
> >
> > — Leif
> >
>
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
I've issued a pull request for the implementation of this:
https://github.com/apache/trafficserver/pull/3145
On Thu, Feb 1, 2018 at 12:49 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
>
>> On Feb 1, 2018, at 9:35 AM, Alan Carroll <so...@oath.com.INVALID> wrote:
>>
>> That's a good suggestion, although shouldn't it be
>>
>> TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc* obj);
>>
>> That is the TSMLoc should be an out parameter, not an in. In that case I'd
>> drop the ...StringGet() and use the generic TSUrlStringGet() if a string is
>> needed.
>
>
> I’m definitely not convinced we need those additional StringGet() APIs, when the existing one would definitely work. As for the unused marshaling buffer, we can look into that for 8.0.0, and decide if it’s never needed, and clean that up and fix the APIs accordingly. I’d much rather prefer that, that introduce new, duplicated functionality APIs.
>
> — Leif
>
Re: Proposed new ts api calls
Posted by Leif Hedstrom <zw...@apache.org>.
> On Feb 1, 2018, at 9:35 AM, Alan Carroll <so...@oath.com.INVALID> wrote:
>
> That's a good suggestion, although shouldn't it be
>
> TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc* obj);
>
> That is the TSMLoc should be an out parameter, not an in. In that case I'd
> drop the ...StringGet() and use the generic TSUrlStringGet() if a string is
> needed.
I’m definitely not convinced we need those additional StringGet() APIs, when the existing one would definitely work. As for the unused marshaling buffer, we can look into that for 8.0.0, and decide if it’s never needed, and clean that up and fix the APIs accordingly. I’d much rather prefer that, that introduce new, duplicated functionality APIs.
— Leif
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
When calling this, where would the value for the bufp parameter come
from? How would it be used in the implementation?
On Thu, Feb 1, 2018 at 10:35 AM, Alan Carroll
<so...@oath.com.invalid> wrote:
> That's a good suggestion, although shouldn't it be
>
> TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc* obj);
>
> That is the TSMLoc should be an out parameter, not an in. In that case I'd
> drop the ...StringGet() and use the generic TSUrlStringGet() if a string is
> needed.
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
That's a good suggestion, although shouldn't it be
TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc* obj);
That is the TSMLoc should be an out parameter, not an in. In that case I'd
drop the ...StringGet() and use the generic TSUrlStringGet() if a string is
needed.
Re: Proposed new ts api calls
Posted by James Peach <jp...@apache.org>.
> On Jan 30, 2018, at 9:15 AM, Walt Karas <wk...@oath.com.INVALID> wrote:
>
> Revised:
>
> const char *TSRemapFromUrlStringGet(const TSHttpTxn txnp, int &length);
>
> const char *TSRemapToUrlStringGet(const TSHttpTxn txnp, int &length);
>
> These would return, from the transaction state object,
> url_map.getFromURL() and url_map.getToURL() respectively, in strong
> format. A null pointer would be returned if the values were empty.
> The length of the string is returned in 'length'. (Nul-termination
> should not be expected/presumed.) . The plugin must release the memory
> for the string by calling TSFree().
This is C, so no reference parameters. Following the existing TSAPI conventions, I'd recommend
TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc obj);
char * TSRemapFromUrlStringGet(TSHttpTxn txnp, int *length);
TSReturnCode TSRemapToUrlGet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc obj);
char * TSRemapToUrlStringGet(TSHttpTxn txnp, int *length);
In the documentation, please be sure to specify which hooks these APIs may be called from. Consider writing an example plugin that can be consumed by tests and included in the documentation.
I'm a not that fazed by the StringGet() APIs proposed here, but we should consider whether we want to continue this API pattern, or defer the stringification to TSUrlStringGet().
> On Mon, Jan 29, 2018 at 4:55 PM, Walt Karas <wk...@oath.com> wrote:
>> The plugin would own them and would have to call TSFree() when done
>> with them. They would be nul terminated.
>>
>> On Mon, Jan 29, 2018 at 4:52 PM, Alan Carroll
>> <so...@oath.com.invalid> wrote:
>>> What are the ownership properties of the memory indicated by the returned
>>> pointers? Are these strings guaranteed to be null terminated or is a length
>>> needed?
>>>
>>> On Mon, Jan 29, 2018 at 3:53 PM, Shu Kit Chan <ch...@gmail.com> wrote:
>>>
>>>> What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
>>>>
>>>> On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid>
>>>> wrote:
>>>>> const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>>>>>
>>>>> const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>>>>>
>>>>> These would return, from the transaction state object,
>>>>> url_map.getFromURL() and url_map.getToURL() respectively, in strong
>>>>> format. A null pointer would be returned if the values were empty.
>>>>
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
I've done a preliminary implementation, based on the version that
returns the URLs as strings:
https://github.com/ywkaras/trafficserver/pull/4
On Tue, Jan 30, 2018 at 6:04 PM, Walt Karas <wk...@oath.com> wrote:
> Another alternative is to get handles to the "binary" URLs:
>
> TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMLoc *url_loc);
>
> TSReturnCode TSRemapToUrlGet(TSHttpTxn txnp, TSMLoc *url_loc);
>
> These would return, from the transaction object state, handles for
> url_map.getFromURL() and url_map.getToURL() respectively.
>
> TSUrlStringGet() could be used to convert these handles to the string
> representation of the URLs. However, TSUrlStringGet() requires a
> TSMBuffer parameter. BUT, TSUrlStringGet() does not actually USE the
> TSMBuffer parameter, it only validates it. So, presumably, I could
> just remove the validation, and pass a dummy value for TSMBuffer.
>
> Curiouser and curiouser.
>
>
> On Tue, Jan 30, 2018 at 1:37 PM, Alan Carroll
> <so...@oath.com.invalid> wrote:
>> I meant this
>>
>> On Tue, Jan 30, 2018 at 11:15 AM, Walt Karas <wk...@oath.com.invalid>
>> wrote:
>>
>>> Revised:
>>> (Nul-termination
>>> should not be expected/presumed.) .
>>
>>
>> vs.
>>
>>
>>> On Mon, Jan 29, 2018 at 4:55 PM, Walt Karas <wk...@oath.com> wrote:
>>> > The plugin would own them and would have to call TSFree() when done
>>> > with them. They would be nul terminated.
>>>
>>
>> I'll got with the revised API as the final, as you indicated.
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
Another alternative is to get handles to the "binary" URLs:
TSReturnCode TSRemapFromUrlGet(TSHttpTxn txnp, TSMLoc *url_loc);
TSReturnCode TSRemapToUrlGet(TSHttpTxn txnp, TSMLoc *url_loc);
These would return, from the transaction object state, handles for
url_map.getFromURL() and url_map.getToURL() respectively.
TSUrlStringGet() could be used to convert these handles to the string
representation of the URLs. However, TSUrlStringGet() requires a
TSMBuffer parameter. BUT, TSUrlStringGet() does not actually USE the
TSMBuffer parameter, it only validates it. So, presumably, I could
just remove the validation, and pass a dummy value for TSMBuffer.
Curiouser and curiouser.
On Tue, Jan 30, 2018 at 1:37 PM, Alan Carroll
<so...@oath.com.invalid> wrote:
> I meant this
>
> On Tue, Jan 30, 2018 at 11:15 AM, Walt Karas <wk...@oath.com.invalid>
> wrote:
>
>> Revised:
>> (Nul-termination
>> should not be expected/presumed.) .
>
>
> vs.
>
>
>> On Mon, Jan 29, 2018 at 4:55 PM, Walt Karas <wk...@oath.com> wrote:
>> > The plugin would own them and would have to call TSFree() when done
>> > with them. They would be nul terminated.
>>
>
> I'll got with the revised API as the final, as you indicated.
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
I meant this
On Tue, Jan 30, 2018 at 11:15 AM, Walt Karas <wk...@oath.com.invalid>
wrote:
> Revised:
> (Nul-termination
> should not be expected/presumed.) .
vs.
> On Mon, Jan 29, 2018 at 4:55 PM, Walt Karas <wk...@oath.com> wrote:
> > The plugin would own them and would have to call TSFree() when done
> > with them. They would be nul terminated.
>
I'll got with the revised API as the final, as you indicated.
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
Revised:
const char *TSRemapFromUrlStringGet(const TSHttpTxn txnp, int &length);
const char *TSRemapToUrlStringGet(const TSHttpTxn txnp, int &length);
These would return, from the transaction state object,
url_map.getFromURL() and url_map.getToURL() respectively, in strong
format. A null pointer would be returned if the values were empty.
The length of the string is returned in 'length'. (Nul-termination
should not be expected/presumed.) . The plugin must release the memory
for the string by calling TSFree().
On Mon, Jan 29, 2018 at 4:55 PM, Walt Karas <wk...@oath.com> wrote:
> The plugin would own them and would have to call TSFree() when done
> with them. They would be nul terminated.
>
> On Mon, Jan 29, 2018 at 4:52 PM, Alan Carroll
> <so...@oath.com.invalid> wrote:
>> What are the ownership properties of the memory indicated by the returned
>> pointers? Are these strings guaranteed to be null terminated or is a length
>> needed?
>>
>> On Mon, Jan 29, 2018 at 3:53 PM, Shu Kit Chan <ch...@gmail.com> wrote:
>>
>>> What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
>>>
>>> On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid>
>>> wrote:
>>> > const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>>> >
>>> > const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>>> >
>>> > These would return, from the transaction state object,
>>> > url_map.getFromURL() and url_map.getToURL() respectively, in strong
>>> > format. A null pointer would be returned if the values were empty.
>>>
Re: Proposed new ts api calls
Posted by Walt Karas <wk...@oath.com.INVALID>.
The plugin would own them and would have to call TSFree() when done
with them. They would be nul terminated.
On Mon, Jan 29, 2018 at 4:52 PM, Alan Carroll
<so...@oath.com.invalid> wrote:
> What are the ownership properties of the memory indicated by the returned
> pointers? Are these strings guaranteed to be null terminated or is a length
> needed?
>
> On Mon, Jan 29, 2018 at 3:53 PM, Shu Kit Chan <ch...@gmail.com> wrote:
>
>> What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
>>
>> On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid>
>> wrote:
>> > const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>> >
>> > const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>> >
>> > These would return, from the transaction state object,
>> > url_map.getFromURL() and url_map.getToURL() respectively, in strong
>> > format. A null pointer would be returned if the values were empty.
>>
Re: Proposed new ts api calls
Posted by Alan Carroll <so...@oath.com.INVALID>.
What are the ownership properties of the memory indicated by the returned
pointers? Are these strings guaranteed to be null terminated or is a length
needed?
On Mon, Jan 29, 2018 at 3:53 PM, Shu Kit Chan <ch...@gmail.com> wrote:
> What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
>
> On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid>
> wrote:
> > const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
> >
> > const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
> >
> > These would return, from the transaction state object,
> > url_map.getFromURL() and url_map.getToURL() respectively, in strong
> > format. A null pointer would be returned if the values were empty.
>
Re: Proposed new ts api calls
Posted by Shu Kit Chan <ch...@gmail.com>.
What does the "I" stand for in "TSIRemapFromUrlStringGet" ?
On Mon, Jan 29, 2018 at 10:02 AM, Walt Karas <wk...@oath.com.invalid> wrote:
> const char *TSIRemapFromUrlStringGet(const TSHttpTxn txnp);
>
> const char *TSIRemapToUrlStringGet(const TSHttpTxn txnp);
>
> These would return, from the transaction state object,
> url_map.getFromURL() and url_map.getToURL() respectively, in strong
> format. A null pointer would be returned if the values were empty.