docker-minecraft-server icon indicating copy to clipboard operation
docker-minecraft-server copied to clipboard

Incorrectly reported application/octet-stream for GENERIC_PACKS using Alpine images

Open Yokutto opened this issue 7 months ago • 9 comments

Enhancement Type

Improve an existing feature

Describe the enhancement

Yep, the title says it all.

Can we consider allowing application/octet-stream content-type for GENERICK_PACK/GENERICK_PACKS?

At current time, it fails with:

[init] Checking if generic packs are up to date
[init] Generic pack(s) are out of date. Re-applying...
[init] [ERROR] Unsupported archive type: application/octet-stream

Yokutto avatar Apr 11 '25 05:04 Yokutto

I saw the source code function that decompress things, and I think it makes sense not allowing application/octet-stream, but can we at least fallback to file extension?

Yokutto avatar Apr 11 '25 05:04 Yokutto

Yes it's a good request. Some web servers certainly fallback to octet stream for zip files.

itzg avatar Apr 11 '25 13:04 itzg

Thanks for considering my suggestion. But, before we implement something like this, we should consider how we will actually do it.

Since application/octet-stream can represent literally anything per definition, we need at least a hint of what file we are dealing with

For this, I think we have two main approaches:

1. Extracting the file extension from the file we downloaded:

I'm not sure if the "download util" follows the filename convention of Content-Disposition header. Also, parsing anything from URLs is too painful. Thinking of that, we should consider extracting the extension from the downloaded file itself, since we will already download it anyways

2. Reading its file signature:

Another option is to examine the file's metadata (its initial bytes) and match them against known file signatures. I think this is the most reliable and consistent method. Unix-based images even include the head command (including Alpine), although I'm not sure if this command is available on the images we are using for this project

Yokutto avatar Apr 11 '25 18:04 Yokutto

You might be overthinking it 😄 If the downloaded file is not a proper zip file, then the zip file extraction will fail since it itself will perform file/header inspection. So, I'd prefer to let the JDK throw the exception rather than jumping through extra complexity to arrive the same outcome: indicating to the user that the file is not a valid zip file. I can have the code catch and massage the JDK level exception into a cleaner, user facing one to clarify the invalid file type.

itzg avatar Apr 11 '25 21:04 itzg

...and now that I look at the source code closer, it's using a file inspection:

https://github.com/itzg/docker-minecraft-server/blob/dcdbb85936ebc7b28d6d4ef5c893043c18f5bf96/scripts/start-utils#L425

to determine the content of the file and has nothing to do with what the web server reported.

Are you certain the file you're referencing is actually a zip (or other compressed) file? Can you provide the URL?

itzg avatar Apr 13 '25 02:04 itzg

Hi, sorry for the late response

So, at the end of the day, that was an even smarter idea than the one I had in mind =P

The generic pack in question is indeed a zip, it contains generic folders from BMC4 serverpack, and you can find it here: https://storage.kazuha.world/.trash/267f4773-5a2f-404f-8441-8cb781b1a2b4.zip

I can extract that zip on pretty much every "unzip" program that I had tested so far, even using the command unzip inside the docker image

But now that's come the weird thing, when testing within alpine images, I was able to reproduce the application/octet-stream that comes out of the command file -b --mime-type "${src}", and I have able to create a very simple docker compose that reproduces the problem:

###
# Environment variables
x-CommonEnvironmentVariables:
  Minecraft: &Minecraft-Variables
    EULA: 'true'
    SETUP_ONLY: 'true'

  Vanilla: &Vanilla-Variables
    <<:
      - *Minecraft-Variables
    TYPE: VANILLA
    GENERIC_PACK: https://storage.kazuha.world/.trash/267f4773-5a2f-404f-8441-8cb781b1a2b4.zip
###

services:
  vanilla.debian:
    image: docker.io/itzg/minecraft-server:java21
    environment:
      <<:
        - *Vanilla-Variables

  vanilla.alpine:
    image: docker.io/itzg/minecraft-server:java21-alpine
    environment:
      <<:
        - *Vanilla-Variables

Sadly, I do not have the time right now to test every image that has been published so far, and I cannot debug in depth why this is happening

And... since this issue doesn't have anything to do with the Content-Type header, I'm really willing to close it now as "wont-fix". I could just use alternative compression algorithms like tar, or just use the debian image. Maybe we just document this somewhere and move on?

Yokutto avatar Apr 16 '25 18:04 Yokutto

Ah, the alpine aspect was important. In the spirit of being minimal, some of their commands are reimplemented to the point of being incomplete (i.e. broken).

Other than quick one-off experiments, I'm really not a fan of Alpine images anymore. In the case of supporting Minecraft and all the automation, the image size savings are not that significant in the end.

itzg avatar Apr 16 '25 21:04 itzg

https://github.com/itzg/docker-minecraft-server/blob/dcf2f46620afbdf0193716d46525228fbcd97fef/scripts/start-utils#L421-L441

needs to be refactored to conditionally look at filename extension when getDistro reports alpine.

itzg avatar Apr 18 '25 13:04 itzg

This issue is stale because it has been open 30 days with no activity. Please add a comment describing the reason to keep this issue open.

github-actions[bot] avatar May 19 '25 02:05 github-actions[bot]

This issue is stale because it has been open 30 days with no activity. Please add a comment describing the reason to keep this issue open.

github-actions[bot] avatar Jun 19 '25 02:06 github-actions[bot]

This issue is stale because it has been open 30 days with no activity. Please add a comment describing the reason to keep this issue open.

github-actions[bot] avatar Jul 20 '25 02:07 github-actions[bot]