apache / trafficserver

Apache Traffic Server™ is a fast, scalable and extensible HTTP/1.1 and HTTP/2 compliant caching proxy server.
https://trafficserver.apache.org/
Apache License 2.0
1.74k stars 782 forks source link

Clean up destination buffer overrun handling in logging unmarshalling. #11437

Closed ywkaras closed 2 weeks ago

ywkaras commented 3 weeks ago

Plus other miscellaneous cleanup/improvement.

JosiahWI commented 3 weeks ago

LSan detected a leak in the ImageMagick plugin.

JosiahWI commented 3 weeks ago

The same failure was repeated.

cmcfarlen commented 3 weeks ago

Would a helper function that takes the format function help clean this up and remove duplication?

ywkaras commented 3 weeks ago

Would a helper function that takes the format function help clean this up and remove duplication?

Would be tricky not to lose the source location (showing which function) in the error message.

ywkaras commented 3 weeks ago

DM from zwoop: it seems in other places where there’s not room, we do no truncate, rather, we just don’t put anything in there. And we certainly never call SiteThrottledError(), that just seems wrong honestly (from what I can tell, SiteThrottledError() is for serious errors around logging completely, disk full) So, I just wouldn’t produce an error here at all (A Dbg() would be more appropriate I think) I’m ok leaving teh truncation if that’s what you prefer Looking more, if you really want a message here, maybe like this: if (res < 0) { SiteThrottledNote("%s", buffer_size_exceeded_msg); (these are incredibly sparsely used though)

cmcfarlen commented 2 weeks ago

Cherry-picked to v10.0.x