Simplify Util#makeResource

[daniel-beck]

This simplifies the code of Util#makeResource by eliminating the creation of a directory hierarchy.

The previous FindBugs suppression explanation was incorrect. Technically this method did have a path traversal issue, but it's not public code and the callers don't invoke it in an unsafe manner. The new code is simpler and easier to reason about.

Testing done

None

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jtnord]

I see this quite a lot presumably to attempt to normalize windows paths.
but Linux paths can (even though it is really really not nornal) have a \ as part of the path.
EXPORT TMP=/tmp/this_is_a_directory_with_a_\\_ok

Happens when the home directory uses a username that comes from samba and using multiple domains and you need to distinguish DOMAIN1\user and DOMAIN2\user and the full domainname e.g. DOMAIN\user and so normalizing this will lead to something unexpected

Suggested change

	File resource = new File(tmpDir.toFile(), getBaseName(name.replace('\\', '/')));
	File resource = new File(tmpDir.toFile(), getBaseName(name.replace(File.separatorChar, '/')));

I don't think you can have a / inside a directory on windows

[daniel-beck]

So this breaks if someone passes in a filename whose last character is \. Unsure whether such callers exist.

Otherwise this will just use a substring of the filename. Since there's one directory per file, there's no risk of collisions, making diagnostics a bit more difficult.

Given existing use of this idiom in the codebase I'm tempted to leave this as is. WDYT?

[jtnord]

So this breaks if someone passes in a filename whose last character is \.

it breaks if someone passes in a name whose filename would contain a \ not necessarily just the last character of the filename when running on non windows OSes.

And by break, it would just mean the returned File does not have the filename that the caller passed in. If this actually breaks anything or not I have no Idea!

Unsure whether such callers exist.

I am not sure what the users of this function are, but I would the case to be exceptionally rare in any case.

Given existing use of this idiom in the codebase

I believe those are no-ops as the path in those is the path within the URL and a URL should even for windows paths use / as separators (file://c:/foo/bar/wibble) and any \ would have to be encoded as %5C

That said you can construct invalid URLs so if they are user supplied or built by hand 🤷

I'm tempted to leave this as is. WDYT?

I'm not blocking this PR on this, feel free to choose whatever path you desire.