doc: undeprecate method getActualClass

[monperrus]

getActualClass was deprecated (I may even be the author of the deprecation).

But:

1. we use it all over the place
2. I used it myself naturally in a transformation I recently wrote.

So it is indeed very useful, and we should undeprecate it.

[I-Al-Istannen]

The method is quite prone to failure:

• It won't do anything useful if you do not have enough of the classpath
• or the names do not line up (e.g. because your classpath contains a different version of the code).
• or the static initializers of the class fail because you did not set up the surrounding context
• or you changed the model, in which case changes are not reflected (you need to recompile the model for it to take effect).

Additonally, it executes arbitrary code of the analyzed project in the context of the input classloader. This poses a significant security risk when analyzing code. With the security manager on its way out you are now basically executing arbitrary code within your security context. I am not sure what spoon's security model is (maybe we don't give any guarantees at all), but this is a worthwhile point to think about here.

Those are quite some red flags to be aware of IMHO.

[monperrus]

Fully agree, hence the API Javadoc:

"This is a low-level feature, it should never been used."

I would also add your wrap-up in the Javadoc. WDYT?

[I-Al-Istannen]

Sounds alright then. It can be useful but you also really need to be careful when using it.

As an aside, formulating some kind of "never run spoon on untrusted code" (or can you?) policy might also be an interesting thing. I am not sure in what contexts spoon is mostly used. I had a few where I absolutely did not trust the code I analyzed but I would have sandboxed the whole thing away.

[monperrus]

updated the javadoc

[monperrus]

Moved the interesting conversation about untrusted code in dedicated issue #4945

[I-Al-Istannen]

That looks a lot better. I would probably expand it to something akin to this. It explicitly specifies a lot of the weird rules spoon follows but clearly marks them as implementation artifacts. We are allowed to break them at any time.

Sorry for being so pedantic here, I really just don't want to debug a bug report involving this method :D

[MartinWitt]

Maybe highlight the risk part a bit more. Personally, I started ignoring warnings like low-level feature, as projects started writing this all over the code.

[monperrus]

feel free to Github-suggest or edit directly :)

[MartinWitt]

How about copying the paragraph from CtTypeReference? I know duplicates are bad, but we really don't change our Javadoc that often.

[monperrus]

I'm in favor of this, this is the best way to get incorrect documentation with drifting.

[MartinWitt]

@I-Al-Istannen it seems like you had a strong opinion on this PR. What is your current state on this PR? Merging, extra refinements, or strongly against it?

[I-Al-Istannen]

#4945 (comment) This basically. After spending an hour trying and failing to exploit it, I am fine with this PR and might actually consider lessening the warning I suggested above a bit. You can still get an arbitrary code execution but it seems you actually have to try.

[MartinWitt]

Thanks, @monperrus and @I-Al-Istannen

[monperrus]

Thanks @MartinWitt @I-Al-Istannen