Remove duplicate items from args, as twitter throws a wobbly otherwise #16

Closed
wants to merge 8 commits into from

2 participants

@castaway

If you manage (somehow ;) to create a URL that contains, for example: &result_type=recent&result_type=recent, then Twitter responds with "Could not authenticate you" .. This makes no sense, but if you remove the duplicate param value, it all works again.

Not sure if this patch is quite sane, but does what I want for now.

@semifor
Owner

I haven't tested this yet, but fear it will break several methods like update_profile_image that expect an arrayref parameter value.

See https://github.com/semifor/Net-Twitter/blob/master/lib/Net/Twitter/Core.pm#L164-171. It's using the underlying file upload support in HTTP::Request::Common. Search the pod for 'Form-based File Upload'.

@castaway

Hmm, tests all pass. Any pointers/hints on how to find out which items I should avoid de-arrayifying? Or I could just collapse the ones that have the same value twice.. Would that do?

@castaway castaway Rewrote the "duplicate keys" patch to:
a) not attempt to collapse "image" and "media" keys (documented as having arrayref args)
b) only collapse arrays with multiple copies of the same value
f25091d
@castaway

I've updated this now to avoid killing the media and image keys (for update_profile_image etc), and to only collapse multiple values with the same content.. Any better?

@castaway

.. I would add a test but.. I don't think the tests actually talk to Twitter itself?

@semifor
Owner

I'm having trouble liking this patch. :) That doesn't mean I'm rejecting it out-of-hand. I want to understand the need for it better.

I think there's yet another place it breaks. Net::Twitter accepts an arrayref for user_id and screen_name. Some methods, like lookup_users expect a comma separated list of IDs. Net::Twitter accepts an arrayref of IDs and flattens them to a comma separated list which seems more perlish to me.

In general, Net::Twitter doesn't pay much attention to the values passed in parameters. Twitter has a habit of adding (and removing) parameters with alarming frequency. With few exceptions, Net::Twitter passes whatever you give it directly through to Twitter. That allows Net::Twitter to accommodate new parameters without requiring a new release.

Perhaps this would be better handled as an optional trait in the Net::Twitter::Role:: namespace. Maybe around _perpare_request.

@castaway

Hmm, guess I can fix it locally as well.. silly Twitter dying inexplicably on silly requests..

@castaway

If we'd like to "fix" it in Net::Twitter as well, imo we need to:
a) Figure out and document which params allow multiple values/arrays.
b) Figure out which params definitely don't, i.e. crash if you send multiple values. So far I've had fun with request_type.

@semifor semifor closed this Mar 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment