Hi Max,
On 3/20/2017 7:18 PM, Weijun Wang wrote:
On 03/19/2017 11:41 PM, Jamil Nimeh wrote:
Hi Max,
Do you know if the MIT krb5 code accepts any filename with the .conf
extension? So filenames with spaces and periods with a .conf suffix are
fine? I just wanted to make sure because your test code doesn't have
any examples that would go outside the old alphanum, +, _, - set of
characters (e.g. "foo.bar yak.config") but should otherwise be OK
because it ends with .conf. If that's the desired behavior then that's
fine, I was more curious than anything else. Maybe not a big deal
because I think even "a.conf" would run down the same codepath as
"foo.bar yak.config".
The MIT krb5 code has
if (len >= 5 && !strcmp(filename + len - 5, ".conf"))
return 1;
So even a bare ".conf" is allowed. My understanding of the old rule is
to exclude OS-generated files like .DS_Store and desktop.ini. Do you
know of any possibilities that a "*.conf" file will be generated this
way?
Honestly, I can't. I could see a sysadmin maybe moving a file like
foo.conf maybe to .foo.conf in order to "hide" it, but that wouldn't do
much now (it will still be processed) and now you have a situation where
the admin has a file being processed that doesn't readily show up in a
simple "ls." Point gun at foot, pull trigger. I don't have a lot of
experience with Kerberos implementations, so I can't think of a case
where the OS would do something like that. At least not for a
system-level config file. Maybe if there was a homedir-based conf
file...sometimes those are made as dot files (e.g. the local .ssh
directory...but that's a directory with non-hidden conf files inside).
Since you're consistent with the MIT stuff, it looks good to me. I was
just curious more than anything else.
As for the test, we can say "k4.conf" already contains "." which was
not allowed before.
Nit: Test code, line 110, looks like there are a couple spaces where
you're chaining methods together that you don't do elsewhere in the
code. Is that intentional?
No. I cannot remember where the spaces come from. Maybe after breaking
and joining lines in vi?
Sounds like a likely explanation to me.
Thanks
Max
Otherwise looks good.
Thanks,
--Jamil
On 3/19/2017 1:35 AM, Weijun Wang wrote:
Please review the code change at
http://cr.openjdk.java.net/~weijun/8177085/webrev.00/
This is to be consistent with MIT krb5 [1]
"Including a directory includes all files within the directory whose
names consist solely of alphanumeric characters, dashes, or
underscores. Starting in release 1.15, files with names ending in
”.conf” are also included."
New case added to test. Also some rename to make it clearer.
Thanks
Max
[1]
http://web.mit.edu/kerberos/krb5-devel/doc/admin/conf_files/krb5_conf.html
