Skip to content

Missing access checks when accessing package private classes #3339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
smarter opened this issue Oct 17, 2017 · 6 comments
Closed

Missing access checks when accessing package private classes #3339

smarter opened this issue Oct 17, 2017 · 6 comments

Comments

@smarter
Copy link
Member

smarter commented Oct 17, 2017

This code shouldn't compile:

package outer {
  private class A

  package inner {
    object Test {
      def main(args: Array[String]): Unit = {
        println(new A)
      }
    }
  }
}

Because if you try to run it you get:

% dotr outer.inner.Test
Exception in thread "main" java.lang.IllegalAccessError: tried to access class outer.A from class outer.inner.Test$
        at outer.inner.Test$.main(i3302.scala:7)
        at outer.inner.Test.main(i3302.scala

This code compiles and run with scalac because the class A is emitted as public whereas dotty emits it as private.

@odersky
Copy link
Contributor

odersky commented Oct 26, 2017

I think the code is legal, so we need to do like scalac and make class A public.

@odersky
Copy link
Contributor

odersky commented Nov 11, 2017

@retronym's comments in #3392 makes a strong case that we should disallow this instead of silently making classes public.

@allanrenucci
Copy link
Contributor

allanrenucci commented Nov 14, 2017

@odersky If I understand correctly, this will break a lot of code. The following pattern will not compile anymore

package collection
package immutable

private[collection] class HashMapCollision1

class Test {
  new HashMapCollision1 // error: not accessible
}

The boundary for HashMapCollision1 is package collection same as if it was defined in package collection and will not be accessible from package immutable.

@odersky
Copy link
Contributor

odersky commented Nov 14, 2017

I also fear it will break a lot of code. @retronym do you think we can pull it off regardless?

Note that -language:Scala2 is too weak here. If we compile a private class with the new scheme, we cannot access it anymore from nested packages even if -language:Scala2 is enabled.

@retronym
Copy link
Member

It would certainly be a big change.

The example above would be best solved by:

package collection {
  package internal {
    class HashMapCollision1
  }
  package immutable {
    object Test { new internal.HashMapCollision1 }
  }
}

In which the JPMS module declaration for scala-library did not export collection.internal outside of the module. Eventually, we'd drop support for Java 8 to assume that the module restrictions are actually enforced.

Alternatively, we could add a different syntax for Java-style package-protected classes. Could we change the meaning of protected, and leave private[p]/protected[p] as is?

@odersky
Copy link
Contributor

odersky commented Jan 2, 2018

Note that private and private[p] are two different mechanisms. We can tighten package access rules for private to exclude nested packages and still keep private[p] as before. We should think about whether we can replace the latter by something like Java's default visibility (I think Kotlin uses internal for this), but that's a separate issue.

Since we did not run into the issue with private before, I think we can risk it tightening the rules.

@smarter smarter closed this as completed in cad41d2 Jan 4, 2018
smarter added a commit that referenced this issue Jan 4, 2018
Fix #3339: Disallow accesses to private package members from nested pkgs
odersky added a commit to dotty-staging/dotty that referenced this issue Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants