Skip to content

Fix ClassesProcessor not whitelisting class files#594

Open
aoqia194 wants to merge 1 commit into
Vineflower:develop/1.13.0from
aoqia194:fix/failed-save
Open

Fix ClassesProcessor not whitelisting class files#594
aoqia194 wants to merge 1 commit into
Vineflower:develop/1.13.0from
aoqia194:fix/failed-save

Conversation

@aoqia194

@aoqia194 aoqia194 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #593!

This PR branch should probably be tested on the CLI against some other projects, because I am not sure if adding this second check will influence anything.

An explanation about the bug

When using --only=com/example, the com/example is marked as a whitelisted prefix; thus any directories in a JAR being matched against these whitelist prefixes will be created. However, when matching a singular class file (e.g. --only=com/example/MyClass), the original check against the to-be-created directory com/example would fail.

The check will boil down to:

"com/example".startsWith("com/example/MyClass")

and it is obvious that this would fail, thus the directory would not be created, and after decompilation happens and the class file is being dumped to a file, it will fail because of the parent directories not existing.

It took me a few hours to find a fix that didn't feel hacky, though I will admit that this still feels a little hacky and might have some unknown-to-me consequences. This simple change will affect this whitelist check and makes the check:

"com/example".startsWith("com/example/MainClass") || "com/example/MainClass".startsWith("com/example")

This check satisfies the directory-creating code and will create the directories just fine. Yippee!

@aoqia194 aoqia194 changed the title Fix whitelisted classes in ClassesProcessor not whitelisting class files Fix ClassesProcessor not whitelisting class files May 22, 2026
@aoqia194

Copy link
Copy Markdown
Contributor Author

I forgot to mention that it's also possible to just check the folders exist again right before writing the file, but I think this might be cheaper anyway? Also it may be better internally that these folders are marked as whitelisted in the first place.

@jaskarth jaskarth added Priority: Medium Medium priority Type: Bugfix Fixes a bug Subsystem: IO Anything concerning the File I/O system labels May 31, 2026
@aoqia194 aoqia194 marked this pull request as draft June 26, 2026 20:27
@aoqia194

Copy link
Copy Markdown
Contributor Author

I'm converting this to draft because this same issue has come up again on this branch despite this fixing it previously?

@aoqia194 aoqia194 marked this pull request as ready for review June 28, 2026 14:56
@aoqia194

Copy link
Copy Markdown
Contributor Author

Ok it seems that this still works after checking, I think it was just user error with my vineflower wrapper. Maybe to prevent false-positives in the whitelisting with this extra check, we can also check for a suffix like .java or .class to determine whether a single file is being whitelisted or not? I don't think it's necessary right now however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: Medium Medium priority Subsystem: IO Anything concerning the File I/O system Type: Bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants