Skip to content

Improve isAccessible checks #1754

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

Merged
merged 1 commit into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class AssembleModelGenerator(private val basePackageName: String) {

return allModificatorsOfClass
.mapNotNull { (fieldId, possibleModificators) ->
chooseModificator(fieldId, possibleModificators)?.let { fieldId to it }
chooseModificator(classId, fieldId, possibleModificators)?.let { fieldId to it }
}
.toMap()
}
Expand All @@ -474,12 +474,13 @@ class AssembleModelGenerator(private val basePackageName: String) {
* Note: direct accessor is more preferred than setter.
*/
private fun chooseModificator(
callerClassId: ClassId,
fieldId: FieldId,
settersAndDirectAccessors: Set<StatementId>,
): StatementId? {
val directAccessors = settersAndDirectAccessors
.filterIsInstance<DirectFieldAccessId>()
.filter {it.fieldId.isAccessibleFrom(basePackageName) }
.filter {it.fieldId.isAccessibleFrom(basePackageName, callerClassId) }

if (directAccessors.any()) {
return directAccessors.singleOrNull()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,26 +323,19 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA
return caller canBeReceiverOf this
}

private fun FieldId.accessSuitability(accessor: CgExpression?): FieldAccessorSuitability {
private fun FieldId.accessSuitability(accessor: CgExpression): FieldAccessorSuitability {
// Check field accessibility.
if (!canBeReadFrom(context)) {
if (!canBeReadFrom(context, accessor.type)) {
return ReflectionOnly
}

// If field is static, then it may not have an accessor.
if (this.isStatic && accessor == null) {
return Suitable
}

if (this.isStatic && accessor != null && codegenLanguage == CodegenLanguage.KOTLIN) {
if (this.isStatic && codegenLanguage == CodegenLanguage.KOTLIN) {
error("In Kotlin, unlike Java, static fields cannot be accessed by an object: $this")
}

// if field is declared in the current test class
if (this.declaringClass == currentTestClass) {
return when {
// field of the current class can be accessed without explicit accessor
accessor == null -> Suitable
// field of the current class can be accessed with `this` reference
accessor isThisInstanceOf currentTestClass -> Suitable
// field of the current class can be accessed by the instance of this class
Expand All @@ -352,10 +345,6 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA
}
}

requireNotNull(accessor) {
"Field access must have a non-null accessor, unless it is the field of the current test class or a static field: $this"
}

if (this.declaringClass == accessor.type) {
// if the field was declared in class `T`, and the accessor is __exactly__ of this type (not a subtype),
// then we can safely access the field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)
for ((index, fieldPathElement) in path.withIndex()) {
when (fieldPathElement) {
is FieldAccess -> {
if (!fieldPathElement.field.canBeReadFrom(context)) {
if (!fieldPathElement.field.canBeReadFrom(context, curType)) {
lastAccessibleIndex = index - 1
break
}
Expand Down Expand Up @@ -246,7 +246,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)

private fun variableForStaticFieldState(owner: ClassId, fieldPath: FieldPath, customName: String?): CgVariable {
val firstField = (fieldPath.elements.first() as FieldAccess).field
val firstAccessor = if (owner.isAccessibleFrom(testClassPackageName) && firstField.canBeReadFrom(context)) {
val firstAccessor = if (owner.isAccessibleFrom(testClassPackageName) && firstField.canBeReadFrom(context, owner)) {
owner[firstField]
} else {
// TODO: there is a function getClassOf() for these purposes, but it is not accessible from here for now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ import org.utbot.framework.plugin.api.util.doubleStreamClassId
import org.utbot.framework.plugin.api.util.doubleStreamToArrayMethodId
import org.utbot.framework.plugin.api.util.intStreamClassId
import org.utbot.framework.plugin.api.util.intStreamToArrayMethodId
import org.utbot.framework.plugin.api.util.isPackagePrivate
import org.utbot.framework.plugin.api.util.isSubtypeOf
import org.utbot.framework.plugin.api.util.longStreamClassId
import org.utbot.framework.plugin.api.util.longStreamToArrayMethodId
Expand Down Expand Up @@ -236,7 +235,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
val accessibleStaticFields = statics.accessibleFields()
for ((field, _) in accessibleStaticFields) {
val declaringClass = field.declaringClass
val fieldAccessible = field.canBeReadFrom(context)
val fieldAccessible = field.canBeReadFrom(context, declaringClass)

// prevValue is nullable if not accessible because of getStaticFieldValue(..) : Any?
val prevValue = newVar(
Expand All @@ -261,7 +260,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
val accessibleStaticFields = statics.accessibleFields()
for ((field, model) in accessibleStaticFields) {
val declaringClass = field.declaringClass
val fieldAccessible = field.canBeSetFrom(context)
val fieldAccessible = field.canBeSetFrom(context, declaringClass)

val fieldValue = if (isParametrized) {
currentMethodParameters[CgParameterKind.Statics(model)]
Expand All @@ -282,7 +281,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte

protected fun recoverStaticFields() {
for ((field, prevValue) in prevStaticFieldValues.accessibleFields()) {
if (field.canBeSetFrom(context)) {
if (field.canBeSetFrom(context, field.declaringClass)) {
field.declaringClass[field] `=` prevValue
} else {
val declaringClass = getClassOf(field.declaringClass)
Expand Down Expand Up @@ -1125,11 +1124,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
private fun FieldId.getAccessExpression(variable: CgVariable): CgExpression =
// Can directly access field only if it is declared in variable class (or in its ancestors)
// and is accessible from current package
if (variable.type.hasField(this)
//TODO: think about moving variable type checks into [isAccessibleFrom] after contest
&& (!isPackagePrivate || variable.type.packageName == context.testClassPackageName)
&& canBeReadFrom(context)
) {
if (variable.type.hasField(this) && canBeReadFrom(context, variable.type)) {
if (jField.isStatic) CgStaticFieldAccess(this) else CgFieldAccess(variable, this)
} else {
utilsClassId[getFieldValue](variable, this.declaringClass.name, this.name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ open class CgVariableConstructor(val context: CgContext) :
// byteBuffer is field of type ByteBuffer and upper line is incorrect
val canFieldBeDirectlySetByVariableAndFieldTypeRestrictions =
fieldFromVariableSpecifiedType != null && fieldFromVariableSpecifiedType.type.id == variableForField.type
if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetFrom(context)) {
if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetFrom(context, obj.type)) {
// TODO: check if it is correct to use declaringClass of a field here
val fieldAccess = if (field.isStatic) CgStaticFieldAccess(fieldId) else CgFieldAccess(obj, fieldId)
fieldAccess `=` variableForField
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.utbot.framework.codegen.util

import org.utbot.framework.codegen.domain.context.CgContext
import org.utbot.framework.plugin.api.ClassId
import org.utbot.framework.plugin.api.CodegenLanguage
import org.utbot.framework.plugin.api.FieldId
import org.utbot.framework.plugin.api.MethodId
Expand All @@ -17,10 +18,19 @@ import org.utbot.framework.plugin.api.util.voidClassId
* which means we can access public, protected and package-private fields
*
* @param context context in which code is generated (it is needed because the method needs to know package and language)
* @param callerClassId object on which we try to access the field
*/
fun FieldId.isAccessibleFrom(packageName: String): Boolean {
fun FieldId.isAccessibleFrom(packageName: String, callerClassId: ClassId): Boolean {
val isClassAccessible = declaringClass.isAccessibleFrom(packageName)
val isAccessibleByVisibility = isPublic || (declaringClass.packageName == packageName && (isPackagePrivate || isProtected))

/*
* There is a corner case which we ignore now.
* Protected fields are accessible in nested classes of inheritors.
*/
val isAccessibleByVisibility =
isPublic ||
isPackagePrivate && callerClassId.packageName == packageName && declaringClass.packageName == packageName ||
isProtected && declaringClass.packageName == packageName
val isAccessibleFromPackageByModifiers = isAccessibleByVisibility && !isSynthetic

return isClassAccessible && isAccessibleFromPackageByModifiers
Expand All @@ -32,14 +42,14 @@ private fun FieldId.canBeReadViaGetterFrom(context: CgContext): Boolean =
/**
* Returns whether you can read field's value without reflection
*/
internal infix fun FieldId.canBeReadFrom(context: CgContext): Boolean {
internal fun FieldId.canBeReadFrom(context: CgContext, callerClassId: ClassId): Boolean {
if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
// Kotlin will allow direct field access for non-static fields with accessible getter
if (!isStatic && canBeReadViaGetterFrom(context))
return true
}

return isAccessibleFrom(context.testClassPackageName)
return isAccessibleFrom(context.testClassPackageName, callerClassId)
}

private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean =
Expand All @@ -48,16 +58,16 @@ private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean =
/**
* Whether or not a field can be set without reflection
*/
internal fun FieldId.canBeSetFrom(context: CgContext): Boolean {
internal fun FieldId.canBeSetFrom(context: CgContext, callerClassId: ClassId): Boolean {
if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
// Kotlin will allow direct write access if both getter and setter is defined
// !isAccessibleFrom(context) is important here because above rule applies to final fields only if they are not accessible in Java terms
if (!isAccessibleFrom(context.testClassPackageName) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) {
if (!isAccessibleFrom(context.testClassPackageName, callerClassId) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) {
return true
}
}

return isAccessibleFrom(context.testClassPackageName) && !isFinal
return isAccessibleFrom(context.testClassPackageName, callerClassId) && !isFinal
}

/**
Expand Down