Re: [PATCH] attr: avoid recursion when expanding attribute macros
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 12, 2025, 10:21 UTC
- Message-ID
- <aRRflKWKpUtfn9tw@pks.im>
- In-Reply-To
- <20251112071651.GB431661@coredump.intra.peff.net>
On Wed, Nov 12, 2025 at 02:16:51AM -0500, Jeff King wrote:
Show 25 quoted lines
> On Wed, Nov 12, 2025 at 07:57:14AM +0100, Patrick Steinhardt wrote: > > > So personally I would've probably leaned into the direction of enforcing > > a hard limit. I don't see a reason why anybody would need more than a > > couple of recursions, it culls both compute and memory growth, and it > > allows us to have a proper error message in case the limit is busted. > > Furthermore, we can demonstrate right now that it wasn't possible to > > have unlimited recursion anyway, which makes it easier to put a new > > limit into place. > > > > But following my above reasoning I think it's okay to turn this into > > iteration, as well, though, but I'd like to hear whether my train of > > thought matches yours. > > Yeah, it does match mine. If I wanted to waste a bunch of CPU and memory > on a hosting site, there are a lot easier ways to do that than with > really long gitattributes. > > I'm not at all opposed to putting in a hard limit on top. My general > feeling is that it never hurts to convert recursion to iteration; it > only gives us more options. I'm not planning to work on a hard limit > myself, but if you want to, be my guest. :) > > I think if we do (or even if we don't), it may also be reasonable to > shrink the max attribute file size to 10MB or even smaller.
I think for now it's okay to convert this into iteration and not introduce a limit, at least as long as we keep an open mind about introducing such a limit in the future. I don't really expect that anyone will ever abuse this, but if I'm wrong and this happens at one point in time we may have to introduce the limit retroactively.
So: I'm happy with your patch, but it might make sense to summarize the discussion in the commit message.
Thanks!
Patrick