Skip to content

Commit 0e11013

Browse files
committed
Make sure we don't load a module but then later can't find it
If a module name contains a '/', it can happen that we find the module (like 'dist/Fstab.lns'), load it and then later can't find it under the invalid module name containing the slash, leading to an endless loop of loding the same module over and over again. We avoid this by making sure there is no '/' in the module name before looking for the module file. Other forms of invalid module names should not cause similar problems as we will simply fail to find the corresponding file. Fixes hercules-team#522
1 parent 0f36085 commit 0e11013

File tree

3 files changed

+45
-0
lines changed

3 files changed

+45
-0
lines changed

src/syntax.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,13 @@ static char *module_filename(struct augeas *aug, const char *modname) {
19621962
char *filename = NULL;
19631963
char *name = module_basename(modname);
19641964

1965+
/* Module names that contain slashes can fool us into finding and
1966+
* loading a module in another directory, but once loaded we won't find
1967+
* it under MODNAME so that we will later try and load it over and
1968+
* over */
1969+
if (index(modname, '/') != NULL)
1970+
goto error;
1971+
19651972
while ((dir = argz_next(aug->modpathz, aug->nmodpath, dir)) != NULL) {
19661973
int len = strlen(name) + strlen(dir) + 2;
19671974
struct stat st;

src/transform.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,9 @@ int transform_validate(struct augeas *aug, struct tree *xfm) {
800800
return 0;
801801
error:
802802
xfm_error(xfm, aug->error->details);
803+
/* We recorded this error in the tree, clear it so that future
804+
* operations report this exact same error (against the wrong lens) */
805+
reset_error(aug->error);
803806
return -1;
804807
}
805808

tests/test-api.c

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,40 @@ static void testLoadBadPath(CuTest *tc) {
735735
aug_close(aug);
736736
}
737737

738+
/* If a lens is set to a partial path which happens to actually resolve to
739+
a file when appended to the loadpath, we went into an infinite loop of
740+
loading a module, but then not realizing that it had actually been
741+
loaded, reloading it over and over again.
742+
See https://github.com/hercules-team/augeas/issues/522
743+
*/
744+
static void testLoadBadLens(CuTest *tc) {
745+
struct augeas *aug;
746+
int r;
747+
char *lp;
748+
749+
// This setup depends on the fact that
750+
// loadpath == abs_top_srcdir + "/lenses"
751+
r = asprintf(&lp, "%s:%s", loadpath, abs_top_srcdir);
752+
CuAssert(tc, "failed to allocate loadpath", (r >= 0));
753+
754+
aug = aug_init(root, lp, AUG_NO_STDINC|AUG_NO_LOAD);
755+
CuAssertPtrNotNull(tc, aug);
756+
CuAssertIntEquals(tc, AUG_NOERROR, aug_error(aug));
757+
758+
r = aug_set(aug, "/augeas/load/Fstab/lens", "lenses/Fstab.lns");
759+
CuAssertRetSuccess(tc, r);
760+
761+
r = aug_load(aug);
762+
CuAssertRetSuccess(tc, r);
763+
764+
// We used to record the error to load the lens above against every
765+
// lens that we tried to load after it.
766+
r = aug_match(aug, "/augeas//error", NULL);
767+
CuAssertIntEquals(tc, 1, r);
768+
769+
free(lp);
770+
}
771+
738772
/* Test the aug_ns_* functions */
739773
static void testAugNs(CuTest *tc) {
740774
struct augeas *aug;
@@ -804,6 +838,7 @@ int main(void) {
804838
SUITE_ADD_TEST(suite, testRm);
805839
SUITE_ADD_TEST(suite, testLoadFile);
806840
SUITE_ADD_TEST(suite, testLoadBadPath);
841+
SUITE_ADD_TEST(suite, testLoadBadLens);
807842
SUITE_ADD_TEST(suite, testAugNs);
808843

809844
abs_top_srcdir = getenv("abs_top_srcdir");

0 commit comments

Comments
 (0)