Forbid the use of APIs to create a temporary file in the standard Java temp folder

Writing anywhere else than the XWiki temporary or permanent folder is a very bad practice.

I just discovered Loading... where I did the mistake (pretty sure I knew it was bad back then, but I still did the mistake), I would like to propose a new Spoon rule to forbid the use of API which makes a bit too easy to make this kind of mistake:

File#createTempFile(String, String)
Files#createTempDirectory(String, FileAttribute<?>...)
Files#createTempFile(String, String, FileAttribute<?>...)

(don’t hesitate if you have others in mind, I will check for stuff like this in Apache commons too)

All those APIs have a version where you can pass a parent folder, so it’s not like it was going to make it harder to code some stuff, it’s just a gentle reminder.

WDYT ?

Here is my +1

+1, thank you!

+1 thanks

I gave this too little time before posting for sure, anyway: doesn’t that API use the environment set temp folder? And couldn’t it be changed in a number of places (config, startup, etc.)?

Asking because two minute searches on the net didn’t bring up reliable / complete docs, but maybe instead of forbidding it, you may configure it to fit XWiki?

Some more:

Files.createTempDir() (from guava)
FileUtils.getTempDirectory() (from apache commons io)

Maybe also fail on System.getProperty("java.io.tmpdir"); since that coud be used to then use the standard apis to create a file in the tmp dir.

+1 (ofc outside of tests)

Thanks

Yes, you’re right, the check will need to ignore the Environment implementation (in AbstractEnvironment.javato be precise, where System.getProperty("java.io.tmpdir"); is used a fallback if the servlet tmp dir cannot be found).

BTW xwiki-commons/xwiki-commons-core/xwiki-commons-cache/xwiki-commons-cache-api/src/main/java/org/xwiki/cache/util/AbstractCacheConfigurationLoader.java at f39cc344f915e450a076cefb100b96202155da42 · xwiki/xwiki-commons · GitHub needs to be checked, it seems to indicate we can have a case where the Environment is not set and we would not use the servlet tmp dir in this case.

This case does not exist in XWiki.

It’s very rarely the same as what the Environment returns. It entirely depends on the application server configuration, and it’s generally the temporary directory of the application server itself so not really what an application should ever use IMO.

Sure, you can generally set it with a Java command line parameter. But I would prefer to not require admins to have to set it to an XWiki related folder as much as possible, and you simply can’t when you have an application server with more than XWiki in it (this system property applies to the whole JVM). An application should use the temporary folder which was allocated to it by the application server, I’m just trying to make sure it’s the case for XWiki.

+1

Thanks,
Marius

Actually, as Thomas mentioned on the xwiki chat, even for tests we’re not supposed to use the system tmp dir, as we’re supposed to use a dir in the target directory and we have tooling for this, see https://dev.xwiki.org/xwiki/bin/view/Community/Testing/JavaUnitTesting/#HTemporaryDirectory