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

[bug] Bad request when trying to use very small avatars/banners #2263

Closed
Craftplacer opened this issue Oct 14, 2023 · 12 comments · Fixed by #2298
Closed

[bug] Bad request when trying to use very small avatars/banners #2263

Craftplacer opened this issue Oct 14, 2023 · 12 comments · Fixed by #2298
Labels
bug Something isn't working

Comments

@Craftplacer
Copy link

Craftplacer commented Oct 14, 2023

Describe the bug with a clear and concise description of what the bug is.

When trying to upload an image that's too small (1x1 image for example) as the avatar or banner the backend just returns Bad Request.

What's your GoToSocial Version?

v0.11.1 git-c7a46e0

GoToSocial Arch

arm64 binary

@Craftplacer Craftplacer added the bug Something isn't working label Oct 14, 2023
@igalic
Copy link
Contributor

igalic commented Oct 15, 2023

how big in bytes are these files?
what, exactly does the backend say, other than bad request?
what do the logs say?

@tsmethurst
Copy link
Contributor

It's probably giving bad request because the image is too small for us to derive the type from the magic bytes. Not 100% sure though, logs would be helpful.

@Craftplacer
Copy link
Author

The only output I see via journalctl is

func=server.glob..func1.Logger.func13.1 level=INFO latency="1.903055ms" method=PATCH statusCode=400 path=/api/v1/accounts/update_credentials msg="Bad Request: wrote 47B"

I used a one-pixel GIF this time, which is 35b.

@tsmethurst
Copy link
Contributor

Could you check the response via the network tab (or equivalent for your browser) when doing an upload via the settings panel? That should give more information, i hope.

@Craftplacer
Copy link
Author

Craftplacer commented Oct 15, 2023

The only "information" I could get from the failed request is this:

{"error":"Bad Request"}

@tsmethurst
Copy link
Contributor

Alright, annoying that it gives no errors, but that's OK. I'm fairly sure it's here where it's going wrong:

// Byte buffer to read file header into.
// See: https://en.wikipedia.org/wiki/File_format#File_header
// and https://github.com/h2non/filetype
hdrBuf := make([]byte, 261)

We use those first 261 bytes as the 'magic' bytes for working out what format a file is. So a 35b 1-pixel gif isn't gonna work there. An 8x8 pixel gif or something, on the other hand, would probably work.

@Craftplacer
Copy link
Author

The problem with this use case of solid colors & efficient image codecs is that no matter what the resolution is, you won't exceed the necessary bytes until you introduce noise or just simply upload an actual complex image.

Reading the README of h2non/filetype: the buffer does not need to be filled, so maybe some kind of IO method is not being used correctly.

@tsmethurst
Copy link
Contributor

Yes probably something like that. We'll take a better look at it when time allows.

@igalic
Copy link
Contributor

igalic commented Oct 16, 2023

Alright, annoying that it gives no errors, but that's OK. I'm fairly sure it's here where it's going wrong:

// Byte buffer to read file header into.
// See: https://en.wikipedia.org/wiki/File_format#File_header
// and https://github.com/h2non/filetype
hdrBuf := make([]byte, 261)

We use those first 261 bytes as the 'magic' bytes for working out what format a file is. So a 35b 1-pixel gif isn't gonna work there. An 8x8 pixel gif or something, on the other hand, would probably work.

it's there a way to make this read graceful?
take what's there, even if it's less than 261 bytes?

@KEINOS
Copy link
Contributor

KEINOS commented Oct 24, 2023

it's there a way to make this read graceful?

I think io.ReadFull is causing the error of unexpected EOF.

// Byte buffer to read file header into.
// See: https://en.wikipedia.org/wiki/File_format#File_header
// and https://github.com/h2non/filetype
hdrBuf := make([]byte, 261)
// Read the first 261 header bytes into buffer.
if _, err := io.ReadFull(rc, hdrBuf); err != nil {
return gtserror.Newf("error reading incoming media: %w", err)
}

How about using copy instead?

func Example() {
	// 1x1 px transparent GIF image
	imgBase64 := "R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="

	imgBin, err := base64.StdEncoding.DecodeString(imgBase64)
	PanicOnErr(err)

	hdrBuf := make([]byte, 261)

	// rc := bytes.NewReader(imgBin)
	// _, err = io.ReadFull(rc, hdrBuf)
	// PanicOnErr(err)

	_ = copy(hdrBuf, imgBin)

	info, err := filetype.Match(hdrBuf)
	PanicOnErr(err)

	fmt.Printf("%s\n", info.MIME.Value)
	// Output:
	// image/gif
}

func PanicOnErr(err error) {
	if err != nil {
		log.Panic(err)
	}
}

If it sounds ok I shall PR.

@tsmethurst
Copy link
Contributor

PR would be great! :)

@KEINOS
Copy link
Contributor

KEINOS commented Oct 25, 2023

@tsmethurst
PR would be great! :)

Done 👍 Hope it helps!

tsmethurst pushed a commit that referenced this issue Oct 25, 2023
* chore: add test of golden cases before fix of #2263

* chore: add test case to reproduce error of #2263

* [bugfix] allow store smaller PNG image than 261 bytes (#2263)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants