fix Issue 313 - Fully qualified names bypass private imports

- b/c we're using a global package tree, imported modules were
  accessible in other scopes using fully qualified names
- maintain a whitelist of imported modules in the current scope

changelog.dd

@@ -1,10 +1,11 @@
 Ddoc
 
-$(COMMENT Pending changelog for 2.070. This will get copied to dlang.org and
-    cleared when master gets merged into stable prior to 2.069.
+$(COMMENT Pending changelog for 2.071. This will get copied to dlang.org and
+    cleared when master gets merged into stable.
 )
 
 $(BUGSTITLE Compiler Changes,
+    $(LI $(RELATIVE_LINK2 imports-313, Import access checks for fully qualified names were fixed.))
 )
 
 $(BUGSTITLE Language Changes,
@@ -13,7 +14,21 @@ $(BUGSTITLE Language Changes,
 )
 
 $(BUGSTITLE Compiler Changes,
+    $(LI $(LNAME2 imports-313, Import access checks for fully qualified names were fixed.)
 
+    $(P It is no longer possible to bypass private imports by using fully qualified names, e.g.
+        the following example will fail with `package std.range is not accessible here`.
+    )
+
+    ---
+    import std.algorithm;
+
+    static assert(std.range.isForwardRange!string);
+    ---
+
+    $(P To ease updating existing code, the old behavior was retained but deprecated.
+    )
+    )
 )
 
 $(BUGSTITLE Language Changes,

src/access.d

@@ -410,3 +410,26 @@ extern (C++) bool checkAccess(Loc loc, Scope* sc, Expression e, Declaration d)
     }
     return false;
 }
+
+/****************************************
+ * Check access to package/module p for scope sc.
+ *
+ * Returns: true if the package is not accessible.
+ *
+ * Because we use a global symbol table tree for imported packages/modules,
+ * access to them needs to be checked based on the imports in the scope chain
+ * (see Bugzilla 313).
+ *
+ */
+extern (C++) bool checkAccess(Loc loc, Scope* sc, Package p)
+{
+    if (sc._module == p)
+        return false;
+    for (; sc; sc = sc.enclosing)
+    {
+        if (sc.scopesym && sc.scopesym.isPackageAccessible(p))
+            return false;
+    }
+    deprecation(loc, "%s %s is not accessible here", p.kind(), p.toPrettyChars());
+    return true;
+}

src/dimport.d

@@ -160,6 +160,8 @@ public:
                         {
                             // mod is a package.d, or a normal module which conflicts with the package name.
                             assert(mod.isPackageFile == (p.isPkgMod == PKGmodule));
+                            if (mod.isPackageFile)
+                                mod.id = p.id; // reuse the same package id
                         }
                     }
                     else
@@ -244,19 +246,42 @@ public:
             // Modules need a list of each imported module
             //printf("%s imports %s\n", sc.module.toChars(), mod.toChars());
             sc._module.aimports.push(mod);
-            if (!isstatic && !aliasId && !names.dim)
+
+            if (!aliasId && !names.dim) // neither a selective nor a renamed import
             {
-                if (sc.explicitProtection)
-                    protection = sc.protection.kind;
+                ScopeDsymbol scopesym;
                 for (Scope* scd = sc; scd; scd = scd.enclosing)
                 {
-                    if (scd.scopesym)
+                    if (!scd.scopesym)
+                        continue;
+                    scopesym = scd.scopesym;
+                    break;
+                }
+
+                if (!isstatic)
+                {
+                    if (sc.explicitProtection)
+                        protection = sc.protection.kind;
+                    scopesym.importScope(mod, Prot(protection));
+                }
+
+                // Mark the imported packages as accessible from the current
+                // scope. This access check is necessary when using FQN b/c
+                // we're using a single global package tree. See Bugzilla 313.
+                if (packages)
+                {
+                    // import a.b.c.d;
+                    auto p = pkg; // a
+                    scopesym.addAccessiblePackage(p);
+                    foreach (id; (*packages)[1 .. packages.dim]) // [b, c]
                     {
-                        scd.scopesym.importScope(mod, Prot(protection));
-                        break;
+                        p = cast(Package) p.symtab.lookup(id);
+                        scopesym.addAccessiblePackage(p);
                     }
                 }
+                scopesym.addAccessiblePackage(mod); // d
             }
+
             mod.semantic();
             if (mod.needmoduleinfo)
             {

src/dmodule.d

@@ -114,12 +114,15 @@ extern (C++) class Package : ScopeDsymbol
 {
 public:
     PKG isPkgMod;
+    uint id;        // auto increment id, used to mask package tree in scopes
     Module mod;     // !=null if isPkgMod == PKGmodule
 
     final extern (D) this(Identifier ident)
     {
         super(ident);
         this.isPkgMod = PKGunknown;
+        __gshared uint packageId;
+        this.id = packageId++;
     }
 
     override const(char)* kind() const
@@ -859,6 +862,7 @@ public:
             p.parent = this.parent;
             p.isPkgMod = PKGmodule;
             p.mod = this;
+            p.id = this.id; // reuse the same package id
             p.symtab = new DsymbolTable();
             s = p;
         }
@@ -889,6 +893,7 @@ public:
                      */
                     pkg.isPkgMod = PKGmodule;
                     pkg.mod = this;
+                    pkg.id = this.id; // reuse the same package id
                 }
                 else
                     error(md ? md.loc : loc, "from file %s conflicts with package name %s", srcname, pkg.toChars());

src/dsymbol.d

@@ -1211,6 +1211,9 @@ private:
     Dsymbols* importedScopes;
     PROTKIND* prots;            // array of PROTKIND, one for each import
 
+    import std.bitmanip : BitArray;
+    BitArray accessiblePackages;// whitelist of accessible (imported) packages
+
 public:
     final extern (D) this()
     {
@@ -1433,6 +1436,19 @@ public:
         }
     }
 
+    final void addAccessiblePackage(Package p)
+    {
+        if (accessiblePackages.length <= p.id)
+            accessiblePackages.length = p.id + 1;
+        accessiblePackages[p.id] = true;
+    }
+
+    final bool isPackageAccessible(Package p)
+    {
+        return p.id < accessiblePackages.length &&
+            accessiblePackages[p.id];
+    }
+
     override final bool isforwardRef()
     {
         return (members is null);

src/dsymbol.h

@@ -288,6 +288,9 @@ class ScopeDsymbol : public Dsymbol
     Dsymbols *importedScopes;   // imported Dsymbol's
     PROTKIND *prots;            // array of PROTKIND, one for each import
 
+    struct BitArray { size_t len; size_t *ptr; };
+    BitArray importTreeMask;
+
 public:
     ScopeDsymbol();
     ScopeDsymbol(Identifier *id);

src/expression.d

@@ -8304,6 +8304,8 @@ public:
                  */
                 if (Declaration d = s.isDeclaration())
                     checkAccess(loc, sc, null, d);
+                if (auto p = s.isPackage())
+                    checkAccess(loc, sc, p);
 
                 // if 's' is a tuple variable, the tuple is returned.
                 s = s.toAlias();

src/module.h

@@ -37,6 +37,7 @@ class Package : public ScopeDsymbol
 {
 public:
     PKG isPkgMod;
+    unsigned id;        // auto increment id for package tree masks
     Module *mod;        // != NULL if isPkgMod == PKGmodule
 
     Package(Identifier *ident);

src/root/array.d

@@ -204,4 +204,10 @@ public:
     {
         return data[0 .. dim];
     }
+
+    extern (D) inout(T)[] opSlice(size_t a, size_t b) inout
+    {
+        assert(a <= b && b <= dim);
+        return data[a .. b];
+    }
 }

test/compilable/imports/a313.d

@@ -0,0 +1,8 @@
+module imports.a313;
+
+// adds private package imports
+private import imports.b313;
+// adds private package core
+private import core.stdc.stdio;
+// adds public alias cstdio
+public alias cstdio = core.stdc.stdio;

test/compilable/imports/b313.d

@@ -0,0 +1,7 @@
+module imports.b313;
+
+void bug()
+{
+    // scope has access to it's own module
+    imports.b313.bug();
+}

test/compilable/imports/pkg313/c313.d

@@ -0,0 +1,5 @@
+module imports.pkg313.c313;
+
+void bug()
+{
+}

test/compilable/imports/pkgmod313/mod.d

@@ -0,0 +1,3 @@
+module imports.pkgmod313.mod;
+
+void bar() {}

test/compilable/imports/pkgmod313/package.d

@@ -0,0 +1,5 @@
+module imports.pkgmod313;
+
+public import imports.pkgmod313.mod;
+
+void foo() {}

test/compilable/test313a.d

@@ -0,0 +1,26 @@
+/*
+REQUIRED_ARGS: -de
+*/
+module test313;
+
+import imports.a313;
+
+void test1()
+{
+    import imports.b313;
+    imports.b313.bug();
+}
+
+void test2()
+{
+    cstdio.printf("");
+}
+
+import imports.pkg313.c313;
+void test3()
+{
+    imports.pkg313.c313.bug();
+}
+
+// private symbols from other modules are still visible
+static assert(core.stringof == "package core");

test/compilable/test313b.d

@@ -0,0 +1,6 @@
+// REQUIRED_ARGS: -de
+void test1()
+{
+    import core.stdc.stdio;
+    core.stdc.stdio.printf("");
+}

test/compilable/test313c.d

@@ -0,0 +1,8 @@
+// REQUIRED_ARGS: -de
+import imports.pkgmod313;
+
+void test()
+{
+    imports.pkgmod313.foo();
+    imports.pkgmod313.bar();
+}

test/compilable/test313d.d

@@ -0,0 +1,9 @@
+// first imported as package
+// EXTRA_SOURCES: imports/pkgmod313/mod.d
+// REQUIRED_ARGS: -de
+import imports.pkgmod313; // then as package module
+
+void test()
+{
+    imports.pkgmod313.foo();
+}

test/compilable/test313e.d

@@ -0,0 +1,9 @@
+// first resolved as package, then created as module (with name package)
+// EXTRA_SOURCES: imports/pkgmod313/mod.d imports/pkgmod313/package.d
+// REQUIRED_ARGS: -de
+import imports.pkgmod313; // then imported as package module
+
+void test()
+{
+    imports.pkgmod313.foo();
+}

test/fail_compilation/fail313.d

@@ -1,19 +1,24 @@
 /*
+REQUIRED_ARGS: -de
 TEST_OUTPUT:
 ---
-fail_compilation/fail313.d(15): Error: function fail313.Derived.str return type inference is not supported if may override base class function
+fail_compilation/fail313.d(16): Deprecation: module imports.b313 is not accessible here
+fail_compilation/fail313.d(23): Deprecation: package core.stdc is not accessible here
+fail_compilation/fail313.d(23): Deprecation: module core.stdc.stdio is not accessible here
 ---
 */
+module test313;
 
-class Base
+import imports.a313;
+
+void test1()
 {
-    abstract int str();
+    imports.b313.bug();
+    import imports.b313;
+    imports.b313.bug();
 }
 
-class Derived : Base
+void test2()
 {
-    override str()
-    {
-        return "string";
-    }
+    core.stdc.stdio.printf("");
 }

test/fail_compilation/imports/a13131checkpoint.d

@@ -6,6 +6,7 @@ auto createGlobalsMixins()              // [4] semantic3
 {
     pragma(msg, "+A");
     enum fullModuleName = "imports.a13131parameters";  // necessary
+    mixin("import "~fullModuleName~";");
     foreach (e ; __traits(derivedMembers, mixin(fullModuleName)))
     {
         // [5] see imports.parameters (it's listed in command line)

test/fail_compilation/imports/a13131parameters.d

@@ -4,6 +4,7 @@ auto createParameterMixins()    // auto is necessary to invoke semantic3 to calc
 {
     pragma(msg, "+B");
     enum fullModuleName = "imports.a13131elec";  // necessary
+    mixin("import "~fullModuleName~";");
     foreach (e ; __traits(derivedMembers, mixin(fullModuleName)))
     {
         // will access yet-not semantic analyzed invalid symbol 'econn' in imports.elec

test/fail_compilation/imports/a313.d

@@ -0,0 +1,7 @@
+module imports.a313;
+
+private import imports.b313;
+private static import imports.b313;
+private static import b313 = imports.b313;
+
+private import core.stdc.stdio;

test/fail_compilation/imports/b313.d

@@ -0,0 +1,4 @@
+module imports.b313;
+
+void bug()
+{}