[reportlab-users] setup.py findFile() logic fragile to symlink loops

[Michał Górny]

Hi,

I've just gotten a report from a Gentoo user that reportlab's setup.py
hangs on build command. After short debugging, we've discovered that
the findFile() function used to look for freetype2 headers is fragile to
symlink loops.

For example, nodejs installs the following symlink:

/usr/include/nodejs/src -> .

Since the function uses os.walk() with follow_symlinks=True, it nests
infinitely while trying to follow the symlink. After adding a print-
debug, we get the following output:

findFile: /usr/include/node
findFile: /usr/include/node/libplatform
findFile: /usr/include/node/src
findFile: /usr/include/node/src/libplatform
findFile: /usr/include/node/src/src
findFile: /usr/include/node/src/src/libplatform
findFile: /usr/include/node/src/src/src
findFile: /usr/include/node/src/src/src/libplatform
findFile: /usr/include/node/src/src/src/src
findFile: /usr/include/node/src/src/src/src/libplatform

and so on. Note that reproducing the issue depends on whether
os.walk() happens to hit the symlink loop before finding freetype2
headers, i.e. on the filesystem path order.

--
Best regards,
Michał Górny

_______________________________________________
reportlab-users mailing list
reportl...@lists2.reportlab.com
https://pairlist2.pair.net/mailman/listinfo/reportlab-users

[Robin Becker]

Hi Michael,

can you try the attached patch. I tested locally with an artificially bad setup with plenty of loops. It failed
initially, but worked with the new code on linux/windows10.

It fixes both findFile and listFiles.

I am isolating with a positive lateral flow today so there might be some obvious flaw.

On 28/03/2022 09:40, Michał Górny wrote:
> Hi,
>
> I've just gotten a report from a Gentoo user that reportlab's setup.py
> hangs on build command. After short debugging, we've discovered that
> the findFile() function used to look for freetype2 headers is fragile to
> symlink loops.
>

........


--
Robin Becker

[Michał Górny]

On Mon, 2022-03-28 at 11:26 +0100, Robin Becker wrote:
> Hi Michael,
>
> can you try the attached patch. I tested locally with an artificially bad setup with plenty of loops. It failed
> initially, but worked with the new code on linux/windows10.
>
> It fixes both findFile and listFiles.
>
> I am isolating with a positive lateral flow today so there might be some obvious flaw.
>

I've asked one of the users reproducing this to test the patch, and he
confirmed that it works fine. Thanks!

That said, I'd personally avoid the 'SD.__self__' part and just kept
the actual list as a variable. I'm not 100% sure but I think this could
qualify relying on implementation details, and therefore could break
at some point in the future.

[Robin Becker]

On 28/03/2022 13:33, Michał Górny wrote:
> On Mon, 2022-03-28 at 11:26 +0100, Robin Becker wrote:

........

>
> I've asked one of the users reproducing this to test the patch, and he
> confirmed that it works fine. Thanks!
>

great

> That said, I'd personally avoid the 'SD.__self__' part and just kept
> the actual list as a variable. I'm not 100% sure but I think this could
> qualify relying on implementation details, and therefore could break
> at some point in the future.
>

Well I asked in the python-dev list to see if this is part of python or not. GvR the former dictator says it's
documented in stdtypes so is OK (part of the standard library). I'll take his word for it and continue to use these
optimizations. At least they're findable through the __self__ usage so fixing won't be a problem when/if they are found
to be bad.

Thanks again for the report
--
Robin Becker