Le 13/04/2021 à 17:45, Tim Düsterhus a écrit :
Christopher,
On 4/13/21 2:41 PM, Christopher Faulet wrote:
Sorry for the delay. I'll comment your patches by replying inline when
No delay experienced. You said that you'd try this week and it's still
this week. So this is fine :-)
appropriate. The quality of the whole series is pretty good. Thanks for
the effort. Just a suggestion to simplify the error handling introduced
in the 7th patch. You may use following prototype for your normalizers:
int uri_normalizer_NAME(const struct ist path, struct ist *dst);
returning for instance 0 on success and anything else on error. This
way, it is easy to return an enum instead of an integer to be able to
handle errors.
Yes, I struggled a bit with the method signature, due to the following
constraints:
1. I did not want to allocate the result buffer within the normalizer
itself, because this makes the method less flexible with regard to both
allocation and freeing.
It is indeed cleaner to allocate it in the caller. We should avoid
responsibility sharing. It is always confusing and leads to bugs.
2. I need to somehow pass a size of the target buffer to prevent buffer
overflows.
Passing a pointer on an ist is a good way to announce the max size when the
normalizer is called and to update it to set the final size of the new
path/query. By contract, the caller must provide an ist owning an allocated buffer.
Thus I can't simply take a `struct ist*` for the destination, as an ist
cannot communicate the size of the underlying buffer. I could
technically take a `struct buffer`, but I'd still like the result to
reside in an ist, because this is what the HTX API expects.
Hum, I don't understand. If you create an ist using the trash buffer this way:
struct ist dst = ist2(replace->area, replace->size);
You can pass a pointer on dst. In the normalizer, you can update its size. It is
thus possible to use dst when calling http_replace_req_path() or
http_replace_req_query().
Your suggested signature would work if I would allocate a trash buffer
within the normalizer. Should I do this? Is it safe to return a pointer
to a trash buffer from a function? I remember something about these
buffers being reused, accidentally destroying the contained information
if one is not careful.
No, it is indeed a very bad idea. the trash buffer must be allocated in the
caller. You already choose the right way on this point. But you can still a
pointer on an ist, locally build from the trash buffer.
--
Christopher Faulet
