On 31 Mar 2014, at 15:37, Alan DeKok <aland@deployingradius.com> wrote:
Arran Cudbard-Bell wrote:
if (fr_print_string(vp->vp_strvalue, vp->length, p, len + 1) != len) { talloc_free(p); fr_strerror_printf("Incorrect size of buffer allocated to hold escaped string"); return NULL;
Except that an embedded zero will end up as 4 characters of "\000". It will *always* hit that error.
The fr_print_string_len function takes that into account, it's like the double pass stuff with aprintf, it's just calculating the length of the buffer required to hold the escaped version of the string.
The xlat code doesn't ask for strings to be escaped because it assumes that the escaping is done elsewhere. That assumption is arguably wrong for embedded NULs.
Agreed.
I think that the xlat code should always escape those characters.
Or ask for those chars to be escaped? Yes.
I'll take a look through the git history to see why xlat behaves this way. The underlying assumptions may no longer be valid.
Well, in 3.x.x there was actually another place where -1 was passed to vp_prints_value, but it was just for getting the value for foreach loops, so i've changed that. For 2.x.x I agree the best fix is just to get fr_prints_value to do the escaping. For 3.x.x I tried modifying the vp_prints_value to do a null terminated memcpy, for quote < 0, but it triggered an assert somewhere deep in the template/ paircmp code. I'll add a note to the issue tracker and deal with it later. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2