Re: [PATCH 08/16] nfsd: escape high characters in binary data

Kees Cook
2019-07-11T01:54:44Z

On Wed, Jul 10, 2019 at 06:09:31PM -0400, J. Bruce Fields wrote:
> On Fri, Jun 28, 2019 at 12:33:58PM -0400, J. Bruce Fields wrote:
> > But I may just give up and go with my existing patch and put
> > off that project indefinitely, especially if there's no real need to fix
> > the existing callers.
>
> I went with the existing patch, but gave a little more thought to
> string_escape_mem. Stuff that bugs me:
>
> - ESCAPE_NP sounds like it means "escape nonprinting
> characters", but actually means "do not escape printing
> characters"
> - the use of the "only" string to limit the list of escaped
> characters rather than supplement them is confusing and kind
> of unhelpful.
> - most of the flags are actually totally unused
>
> So what I'd like to do is:
>
> - eliminate unused flags
> - use the "only" string to add to, rather than replace, the list
> of characters to escape
> - separate flags into those that select which characters to
> escape, and those that choose the format of the escaping ("\ "
> vs "\x20" vs "\040".)
>
> I've got some patches that do all that and I think it works. I need to
> clean them up a bit and fix up the tests.

This sounds amazing; thanks! Luckily there are self-tests for this code,
so anything really surprising should stand out. I'm looking forward to
it -- I want to see if I can refactor a few of the callers (if you
haven't already do so) too.

Yay!

--
Kees Cook