Remove hard-coded default for RHS #10
Conversation
|
LGTM |
|
Hm, this seems to break the tests @nalind Can you take a look at the failure? |
|
Would it make more sense to continue setting a default LHS, erroring out if we can't read an RHS and also don't find one in the environment? Setting HES_DOMAIN during the tests should be enough for them. |
|
Okay, giving that a try. |
nalind
changed the title
nalind
force-pushed
the
no_def_lhsrhs
branch
3 times, most recently
from
362b76d to
2e8be6a
Compare
|
Okay, switching back to setting a default LHS if we fail to read the configuration file, and erroring out later if there isn't an acceptable RHS value set in the environment, and making sure that HES_DOMAIN is set to "athena.mit.edu" when we run hestest, seems to work. |
| /* Make sure that the rhs is set. */ | ||
| if (!ctx->rhs) | ||
| { | ||
| errno = ENOEXEC; |
There was a problem hiding this comment.
It's copied from read_config_file(), though I have no idea why read_config_file() uses that particular error code. Would ENOENT make more sense?
There was a problem hiding this comment.
EINVAL or ENOENT make more sense
There was a problem hiding this comment.
Okay, I'm leaning toward EINVAL because at that point we may well have managed to read a configuration file, so ENOENT could be confusing.
Don't fall back to using a default LHS or RHS when the configuration file can't be read. Instead, return an error. Original report from https://bugzilla.redhat.com/show_bug.cgi?id=1332493
|
Hmm, the more I think about this, the more I'm leaning toward going back to just returning an error when we fail to open the file, and setting HESIOD_CONFIG to point to the in-tree hesiod.conf.sample file when we run hestest. That seems to be enough for it here. Opinions? |
|
I would be okay with pointing hestest at a config file. That seems most correct if you're improving the unconfigured state. |
|
Okay, I'll switch this over to doing that. |
|
Yes, having the tests point to an explicit config file and not relying on default behavior seems like a fine plan. |
|
Okay, the changes are ready. |
|
This has been assigned CVE-2016-10152 |
Don't fall back to using a default LHS or RHS when the configuration file can't be read. Original report from https://bugzilla.redhat.com/show_bug.cgi?id=1332493