apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Garrett Rooney <roo...@electricjellyfish.net>
Subject Re: [PATCH] avoid crashing when given invalid user/group ids on win32
Date Sun, 22 Jan 2006 19:14:06 GMT
On 1/22/06, William A. Rowe, Jr. <wrowe@rowe-clan.net> wrote:
> Garrett Rooney wrote:
> > The fix is to use IsValidSid to confirm the validity of the uid/gid
> > before we try to call LookupAccountSid.
> I'd disagree.  The test tried to force a hardcode platform specific value
> at apr_uid_get etc... that's invalid and should crash the users' code.
> We have a general principal in apr that faulty code produces crashes, while
> faulty data produces errors.  I'd disagree that the data was faulty.
> What uid/gid were they trying to test?  Should we have some static helpers
> to get the root / everyone sorts of ID's on a platform-by-platform basis?

The test is trying to send a nonexistant UID/GID (it hardcodes 9999999
as the value, FWIW), that seems like "faulty data" to me.  If you
aren't allowed to just throw random crap at these functions that's
also fine, but given that we actually have a test for that behavior it
sure implies that it's allowed.  If not, the test should go away and
we should probably document that these values are platform specific,
and that the only portable way to get one is via an APR function that
goes from name to UID or name to GID.

> > The one thing I'm really not sure of is what should be done on non-NT
> > systems.  The MSDN docs say that IsValidSid didn't show up until NT
> > workstation 3.1.
> Rule of thumb; Win95 is bare minimum baseline, and we aren't really even
> trying to support anything pre-WinNT (although if folks continue to offer
> patches for 9x series platforms I guess they will always be welcomed while
> we can apply them without hurting a more sophisticated implementation.)

Considering that other functions we use in this code also have the
same "when did it show up" language in the MSDN docs I don't think
it's a reason not to use the code.


View raw message