Skip to content

Commit aa636ce

Browse files
committed
Fix leaking of types and enums from other imported specs to specs which does not import them explicitly
_(review in whitespace ignored mode)_
1 parent 54524e1 commit aa636ce

File tree

3 files changed

+46
-18
lines changed

3 files changed

+46
-18
lines changed

shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ case class ClassSpec(
8181

8282
var seqSize: Sized = NotCalculatedSized
8383

84+
/** The list of top-level type specifications which contains import of this type. */
85+
var importedInto = mutable.ListBuffer[ClassSpec]()
86+
8487
def toDataType: DataType = {
8588
val cut = CalcUserType(name, None)
8689
cut.classSpec = Some(this)

shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class LoadImports(specs: ClassSpecs) {
3232
loadImport(
3333
name,
3434
curClass.meta.path ++ List("imports", idx.toString),
35-
Some(curClass.fileNameAsStr),
35+
curClass,
3636
workDir
3737
)
3838
}).map((x) => x.flatten)
@@ -44,9 +44,10 @@ class LoadImports(specs: ClassSpecs) {
4444
Future.sequence(List(thisMetaFuture, nestedFuture)).map((x) => x.flatten)
4545
}
4646

47-
private def loadImport(name: String, path: List[String], inFile: Option[String], workDir: ImportPath): Future[List[ClassSpec]] = {
47+
private def loadImport(name: String, path: List[String], curClass: ClassSpec, workDir: ImportPath): Future[List[ClassSpec]] = {
4848
Log.importOps.info(() => s".. LoadImports: loadImport($name, workDir = $workDir)")
4949

50+
val inFile = Some(curClass.fileNameAsStr)
5051
val impPath = ImportPath.fromString(name)
5152
val fullPath = ImportPath.add(workDir, impPath)
5253

@@ -63,8 +64,9 @@ class LoadImports(specs: ClassSpecs) {
6364
s".. LoadImports: loadImport($name, workDir = $workDir), got spec=$specNameAsStr"
6465
})
6566
optSpec match {
66-
case Some(spec) =>
67-
val specName = spec.name.head
67+
case Some(importedSpec) =>
68+
importedSpec.importedInto += curClass
69+
val specName = importedSpec.name.head
6870
// Check if spec name does not match file name. If it doesn't match,
6971
// it is probably already a serious error.
7072
if (name != specName)
@@ -88,12 +90,12 @@ class LoadImports(specs: ClassSpecs) {
8890
val isNewSpec = specs.synchronized {
8991
val isNew = !specs.contains(specName)
9092
if (isNew) {
91-
specs(specName) = spec
93+
specs(specName) = importedSpec
9294
}
9395
isNew
9496
}
9597
if (isNewSpec) {
96-
processClass(spec, ImportPath.updateWorkDir(workDir, impPath))
98+
processClass(importedSpec, ImportPath.updateWorkDir(workDir, impPath))
9799
} else {
98100
Log.importOps.warn(() => s"... we have that already, ignoring")
99101
Future { List() }

shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ import io.kaitai.struct.problems._
99
/**
1010
* A collection of methods that resolves user types and enum types, i.e.
1111
* converts names into ClassSpec / EnumSpec references.
12+
*
13+
* This step runs for each top-level [[format.ClassSpec]].
1214
*/
1315
class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) extends PrecompileStep {
16+
/** Resolves references to types and enums in `topClass` and all its nested types. */
1417
override def run(): Iterable[CompilationProblem] =
1518
topClass.mapRec(resolveUserTypes).map(problem => problem.localizedInType(topClass))
1619

@@ -68,6 +71,14 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
6871
}
6972
}
7073

74+
/**
75+
* Resolves `typeName` reference used in `curClass` to a type definition, or
76+
* returns [[TypeNotFoundErr]] error.
77+
*
78+
* @param curClass Class that contains member
79+
* @param typeName A reference to a type that need to be resolved
80+
* @param path A path to the attribute in KSY where the error should be reported if reference is unknown
81+
*/
7182
def resolveUserType(curClass: ClassSpec, typeName: List[String], path: List[String]): (Option[ClassSpec], Option[CompilationProblem]) = {
7283
val res = realResolveUserType(curClass, typeName, path)
7384

@@ -103,7 +114,7 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
103114
// No further names to resolve, here's our answer
104115
Some(nestedClass)
105116
} else {
106-
// Try to resolve recursively
117+
// Try to resolve recursively in all nested classes
107118
realResolveUserType(nestedClass, restNames, path)
108119
}
109120
)
@@ -124,12 +135,18 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
124135
// If there's None => no luck at all
125136
val resolvedTop = specs.get(firstName)
126137
resolvedTop match {
127-
case None => None
128-
case Some(classSpec) => if (restNames.isEmpty) {
129-
resolvedTop
130-
} else {
131-
realResolveUserType(classSpec, restNames, path)
138+
// We should use that spec if it is imported in our file (which is represented
139+
// by our top-level class). It `topClass` imports `classSpec`, we could try to
140+
// resolve type in it
141+
// TODO: if type is defined in spec, we could add a suggestion to error to add missing import
142+
case Some(classSpec) if (classSpec.importedInto.contains(topClass)) => {
143+
if (restNames.isEmpty) {
144+
resolvedTop
145+
} else {
146+
realResolveUserType(classSpec, restNames, path)
147+
}
132148
}
149+
case _ => None
133150
}
134151
}
135152
}
@@ -183,13 +200,19 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
183200
// If there's None => no luck at all
184201
val resolvedTop = specs.get(firstName)
185202
resolvedTop match {
186-
case None => None
187-
case Some(classSpec) => if (restNames.isEmpty) {
188-
// resolved everything, but this points to a type name, not enum name
189-
None
190-
} else {
191-
resolveEnumSpec(classSpec, restNames)
203+
// We should use that spec if it is imported in our file (which is represented
204+
// by our top-level class). It `topClass` imports `classSpec`, we could try to
205+
// resolve type in it
206+
// TODO: if type is defined in spec, we could add a suggestion to error to add missing import
207+
case Some(classSpec) if (classSpec.importedInto.contains(topClass)) => {
208+
if (restNames.isEmpty) {
209+
// resolved everything, but this points to a type name, not enum name
210+
None
211+
} else {
212+
resolveEnumSpec(classSpec, restNames)
213+
}
192214
}
215+
case _ => None
193216
}
194217
}
195218
}

0 commit comments

Comments
 (0)