Add missing checks to some newPostSchema fields

[jonbarrow]

Resolves #XXX

Changes:

• Adds .base64() Zod checks to app_data, painting, and screenshot
• Adds a length check to app_data

The appdata is capped at 1kb officially but this wasn't being enforced. Only realized this while looking at something in Shovel Knight related to Miiverse, was this known before? Or does it need to be documented still? I got the real name from some debug logs in Shovel Knight.

Originally I was only adding the 1kb size check, but then I noticed that the painting and screenshot fields also lacked the base64 check at the Zod level so I tacked those on while I was here. This does, however, make these 3 replace calls redundant:

const painting = bodyCheck.data.painting?.replace(/\0/g, '').trim() || '';
const screenshot = bodyCheck.data.screenshot?.replace(/\0/g, '').trim() || '';
const appData = bodyCheck.data.app_data?.replace(/[^A-Za-z0-9+/=\s]/g, '').trim() || '';

Since Zod will throw an error in cases where the base64 is invalid. These 3 lines look smelly to me anyway, have we seen official clients send badly formatted data before? I can't remember if these 3 lines are required or just defensive. If they are required and the official client does send that badly formatted data, then the .base() checks will need to be removed from the Zod level otherwise """good""" data will never reach this stage. But I find it odd that the official client would send bad data like this? Pinging @ashquarky and @CaramelKat for input there.

• I have read and agreed to the Code of Conduct.
• I have read and complied with the contributing guidelines.
• What I'm implementing was an approved issue.
• I have tested all of my changes. (I tested the changes in a control script, not using an actual client, since the changes were so simple. Hence the possibility of the .base() needing to be removed. But the length check matches what real clients do so it should be fine here too)

[mrjvs]

The checks originate from these commits:

• PretendoNetwork/miiverse-api@a0a5e3d
• PretendoNetwork/miiverse-api@98bc104

They are quite old and doesnt seem to result from a bugfix, so maybe it was indeed defensive?

[jonbarrow]

Yeah it seems likely to be defensive. I'll let the others verify though just to be safe.

I might mark this as a draft as well, since I'm actually seeing a number of other odd areas. For example:

if (messageBody && messageBody.length > 280) {
	request.log.warn('Message body too long');
	return badRequest(response, ApiErrorCode.BAD_PARAMS);
}

Where 280 seems rather arbitrary? I've looked at the official client and it looks like text bodies are actually capped to 255 UTF16 characters, not 280 (there's actually 2 different "max lengths" being checked, which have 2 different values, but the other one doesn't matter in this case). I'm debating marking this as a draft and just trying to fix as many of these little things as I can in one go

[mrjvs]

I'd advise making a new PR for the other odds things you find/fix. Smaller PRs are nicer to work with.

[jonbarrow]

I agree with smaller PRs being better (that's why I put it in the contribution guide), though I'm not sure how large that would make the PR or if it would be considered not small, plus these are all related changes? I'm not sure that many (effectively single-change) PRs all touching the same logic (post input validation) is inherently better than one bigger PR that covers all the logic?

I don't really care either way though, it's no skin off my back however it goes, just my 2 cents. I'll happily just make multiple PRs