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

validateCreateStatus: Check the presence of form.MediaIDs more thoroughly, instead of just checking for null #766

Merged

Conversation

blackle
Copy link
Contributor

@blackle blackle commented Aug 26, 2022

awesome software! just set up a test instance tonight and ran into this snag. It seems that polls aren't really supported yet, but to get it to work eventually this change will probably be necessary.

When sending a post on tusky, it appears that "form.MediaIDs" isn't nil but instead an empty array. If you try to attach a poll, the post is rejected because this code thinks there's both a poll and media on the status.

On pinafore, this doesn't occur (though the post is still rejected because the expires_in field of the payload is a string and not an int. different problem that probably needs to be fixed in pinafore)

@tsmethurst
Copy link
Contributor

Thanks for the PR! Did you test this on your instance yet? Reason I'm asking is because the nil value for a []string should (iirc) just be an empty slice anyway, since it's not a pointer, so I don't think the length check is necessary. Though I could well be wrong, I very often get these things mixed up :P

@NyaaaWhatsUpDoc
Copy link
Member

NyaaaWhatsUpDoc commented Aug 26, 2022

Yes @tsmethurst is correct you don't need a nil check on the media IDs. A nil slice returns a length of zero (no panic). Though I think the replacement of of the nil check with the length check is a welcome improvement, it catches cases of zero length (non-nil) slices :)

@blackle
Copy link
Contributor Author

blackle commented Aug 26, 2022

ok, just updated it to check the length only. tested on my instance with both pinafore and tusky. it still rejects polls from pinafore, but that is because the expires_in field is an unexpected type (string instead of int)

@tsmethurst tsmethurst merged commit e9b5ba0 into superseriousbusiness:main Aug 26, 2022
@tsmethurst
Copy link
Contributor

tsmethurst commented Aug 26, 2022

Merged, thank you! :) Re: expires_in, I guess that's something we can address when we add proper poll support, since submitting a poll won't work rn anyway even if that field is fixed. Looks like the Mastodon API probably accepts either string or number for that, though only number is mentioned in the api: https://docs.joinmastodon.org/methods/statuses/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants