Masaru Tsuchiyama wrote on Sun, Aug 18, 2013 at 22:13:54 +0900:
> Daniel Shahaf wrote:
>> I think that's the wrong fix. Input validation should be done by
>> checking that the input is valid, not by ruling out every known invalid
>> value.[1] In this case: by checking os.path.exists() at the point in the
>> code that tries to interpret the argument as a filename.
>
> Fixed in the attached patch.
>
> Regards.
>
> --
> Masaru Tsuchiyama <m.tmatma_at_gmail.com>
> Index: gen-make.py
> ===================================================================
> --- gen-make.py (revision 1515099)
> +++ gen-make.py (working copy)
> @@ -278,6 +278,9 @@ if __name__ == '__main__':
> except getopt.GetoptError, e:
> _usage_exit(str(e))
>
> + if args and args[0] and os.path.exists(args[0]) != True:
You shouldn't compare to True or to False. Just use the value as
a boolean:
if args and args[0] and not os.path.exists(args[0]):
> + _usage_exit("argument must be a path to build.conf file")
> +
> conf = 'build.conf'
> skip = 0
> gentype = 'make'
This fix doesn't handle permission problems (which I mentioned in my
previous mail). I think the easiest way to handle those (as well as any
other, unforeseen problems that .read() might run into and sweep under
the rug) is to open the file ourselves:
[[[
Index: build/generator/gen_base.py
===================================================================
--- build/generator/gen_base.py (revision 1515085)
+++ build/generator/gen_base.py (working copy)
@@ -75,7 +75,7 @@ class GeneratorBase:
# Now read and parse build.conf
parser = configparser.ConfigParser()
- parser.read(fname)
+ parser.readfp(open(fname))
self.conf = build_path(os.path.abspath(fname))
]]]
[[[
% ./gen-make.py $'\n'
Traceback (most recent call last):
...
IOError: [Errno 2] No such file or directory: '\n'
]]]
I've committed that.
Thanks for your patch!
Daniel
Received on 2013-08-18 15:31:25 CEST