sbctl icon indicating copy to clipboard operation
sbctl copied to clipboard

sbctl bundle splash image should detect if it's a valid bitmap file

Open Foxboron opened this issue 3 years ago • 13 comments

Currently we accept any file and it's not clear why the image isn't displayed. We should probably inspect the file and ensure it's valid before we include it.

Some details of the format can be found here.

https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-screen-components#add-the-logo-to-the-bgrt

Foxboron avatar May 20 '21 22:05 Foxboron

Probably magick identify is a good tool for this.

igo95862 avatar May 21 '21 14:05 igo95862

I'd prefer if we managed to write some Go code for this :) I dislike shelling out for tools and this could either be it's own library, find something already implemented or we make it a thing in go-uefi.

https://github.com/Foxboron/go-uefi

Foxboron avatar May 21 '21 14:05 Foxboron

Go based BMP decoder/encoder:

https://pkg.go.dev/golang.org/x/image/bmp

I dislike shelling out for tools

I also dislike spawning processes when a complex output parser is required but image magick exact output can be specified which makes parsing trivial.

igo95862 avatar May 21 '21 14:05 igo95862

@Foxboron what do you think about temporary solution with magick identify.

It can be an optional dependency. If its not found on the system when a warning will be printed that splash image could not be verified.

igo95862 avatar Jun 01 '21 07:06 igo95862

If we are only detecting the filetype pulling imagemagick is a bit much I think, we only really need to peek at 2 bytes in the file to see if it's a BMP (Example). But if you want to try and use more of the output to check towards the accepted image format then It's probably fine :)

Foxboron avatar Jun 06 '21 11:06 Foxboron

But if you want to try and use more of the output to check towards the accepted image format then It's probably fine

I will do that.

Imagemagick also verifies that the image is valid.

When should image check happen? In GenerateBundle ?

Should it instantly return error or just skip image?

igo95862 avatar Jun 06 '21 17:06 igo95862

Maybe also check in bundle command?

igo95862 avatar Jun 06 '21 17:06 igo95862

I think we can just do a hard failure in the bundle command when there is a file passed there :) No need for one in GenerateBundle as we should probably assume the values have been validated.

Foxboron avatar Jun 06 '21 17:06 Foxboron

How should lack of image magick be handled? Maybe print a warning that image could not be verified but proceed?

igo95862 avatar Jun 06 '21 17:06 igo95862

If the file extension is not ".bmp" just say it can't be verified. Good enough I think

Foxboron avatar Jun 06 '21 17:06 Foxboron

I mean the magick command not the magic numbers.

igo95862 avatar Jun 06 '21 17:06 igo95862

Yep, I got that :) But a simple fil extension check if fine if imagemagick is not available I think.

Foxboron avatar Jun 06 '21 18:06 Foxboron

I made initial check function but it far from finished: https://github.com/igo95862/sbctl/commit/577c1828bd2f726bcd2096773d22fb7980f7379c

igo95862 avatar Jun 06 '21 19:06 igo95862