Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Images are not usable after downloading them using itzg/docker-minecraft-server #114

Closed
loonsies opened this issue Feb 8, 2024 · 8 comments
Labels
status: confirmed This issue has been confirmed and waiting for a resolution type: bug Something isn't working

Comments

@loonsies
Copy link

loonsies commented Feb 8, 2024

Hello,
I have a server hosted in a docker-compose using the itzg/docker-minecraft-server image
When I download images it works and places them correctly in yamipa's images folder, but then when using /image give or /image list the image simply doesn't exists
image
image

Any idea what's going on here? the data folder is mounted between the host and the container, but that shouldn't be an issue...
thanks

@loonsies
Copy link
Author

loonsies commented Feb 10, 2024

Looks like it works now, the image would only show up in the list after rebooting the container.
I don't know if this plugin listen for system events for new files or if it's checked every time commands are called, maybe that's a clue?
image
I tried checking file permissions and ownership without restarting the container (red arrow) but looks they're all the same.

@josemmo
Copy link
Owner

josemmo commented Feb 12, 2024

Hello @senolem,

Could you set verbose: true in the Yamipa configuration file and paste the server logs here?

@josemmo josemmo added type: bug Something isn't working status: info needed More information is needed to review the issue labels Feb 12, 2024
@loonsies
Copy link
Author

loonsies commented Feb 13, 2024

image

nothing interesting...

but something is odd. it can detect that there is already a file with the same name

image

@loonsies
Copy link
Author

loonsies commented Feb 13, 2024

by reading the code, my guess would be that something is going on with the destPath we use for the new image.
I did not edited the images-path value, neither allowed-paths
It would be interesting to see what destPath looks like in the downloadImage() function.
What is odd is that it would mean storage.isPathAllowed() does not return false when downloading an image, but does when trying to fetch the file in the ImageFileArgument class when using /image place, and when fetching image names in getFilenames(), called when using /image list

If that's the case, I would suggest displaying a warning when an unallowed path stored in the files array is detected, for example in getFilenames() we should display a warning if isPathAllowed(filename, sender) returns false.

Could you give me directions on how to build the plugin? I never coded java before.

@josemmo
Copy link
Owner

josemmo commented Feb 13, 2024

You can build the project by cloning the repository and opening it in IntelliJ IDEA. Then, click on the Maven tab on right on the screen and double click "Lifecycle" > "Package". This should have created a JAR file on the "target" directory inside the project.

I don't think it's related to the ImageStorage#isPathAllowed() method, as you should have seen debug logs on the server console at download regardless of your plugin configuration.

This must have something to do with Java's WatchService class and inotify on Linux (see this link). When I find some time, I'll try to reproduce the bug on a container.

@loonsies
Copy link
Author

loonsies commented Feb 13, 2024

Thanks I managed to build it.

if (isPathAllowed(filename, sender)) { response.add(filename); } else { LOGGER.warning("Unallowed path detected in the registered files map"); }

I tried to add a warning but it doesn't show up, sorry for the wrong assumption.

@josemmo josemmo added status: confirmed This issue has been confirmed and waiting for a resolution and removed status: info needed More information is needed to review the issue labels Mar 28, 2024
@josemmo
Copy link
Owner

josemmo commented Mar 28, 2024

Hi, @senolem!

Could you verify that this issue is now fixed in the v1.3.2-SNAPSHOT pre-release version of Yamipa?

@RedstoneFuture
Copy link

RedstoneFuture commented Oct 5, 2024

@senolem
Would you like to test it or write the status here? (with Minecraft version <1.20.6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed This issue has been confirmed and waiting for a resolution type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants