Skip Menu |

This queue is for tickets about the MediaWiki-API CPAN distribution.

Report information
The Basics
Id: 70428
Status: resolved
Priority: 0/
Queue: MediaWiki-API
People
Owner: Nobody in particular
Requestors: DOHERTY [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: Important
Broken in: 0.37
Fixed in: (no value)

History Show all quoted text
  Mon Aug 22 19:36:14 2011 DOHERTY [...] cpan.org - Ticket created
Subject: Don't cause spurious warnings in URI::Escape
To avoid spurious warnings in URI::Escape, please do the following in _make_querystring: sub _make_querystring { my ($ref) = @_; my @qs = (); my $keyval; for my $key ( keys %{$ref} ) { - $keyval = uri_escape_utf8($key) . '=' . uri_escape_utf8($ref->{$key}); + $keyval = uri_escape_utf8($key) . '=' . uri_escape_utf8($ref->{$key} || ''); push(@qs, $keyval); } return '?' . join('&',@qs); } This provides a defined value so URI::Escape doesn't whinge.
  Fri Aug 26 16:06:05 2011 buzz [...] exotica.org.uk - Correspondence added
On Mon Aug 22 19:36:14 2011, DOHERTY wrote: Show quoted text
> To avoid spurious warnings in URI::Escape, please do the following in > _make_querystring:
this would suggest you are passing undefined parameters into the module though ? If i did a check for this, it would make more sense perhaps to return an error as it would point to a problem with the calling code? unless you have a use case i've missed?
  Fri Aug 26 16:06:06 2011 The RT System itself - Status changed from 'new' to 'open'
  Fri Aug 26 16:10:58 2011 DOHERTY [...] cpan.org - Correspondence added
On Fri Aug 26 16:06:05 2011, exobuzz wrote: Show quoted text
> On Mon Aug 22 19:36:14 2011, DOHERTY wrote:
> > To avoid spurious warnings in URI::Escape, please do the following in > > _make_querystring:
> > this would suggest you are passing undefined parameters into the module > though ? If i did a check for this, it would make more sense perhaps to > return an error as it would point to a problem with the calling code? > > unless you have a use case i've missed?
Is there something wrong with query strings like '? thing=&otherthing=hasavalue'? If that's valid, then what's the point in forcing the caller to pass '' instead of undef? That seems to be making them jump through hoops for no reason.
  Fri Aug 26 16:12:47 2011 buzz [...] exotica.org.uk - Correspondence added
Show quoted text
> Is there something wrong with query strings like '? > thing=&otherthing=hasavalue'? If that's valid, then what's the point in > forcing the caller to pass '' instead of undef? That seems to be making > them jump through hoops for no reason.
whats the problem with passing '' instead of undef? since parameters are passed as a hashref, it makes sense if an empty value is wanted to pass an empty value. something= in a querystring is not undefined. its an empty value.
  Fri Aug 26 16:15:59 2011 buzz [...] exotica.org.uk - Correspondence added
however, instead of arguing semantics, i guess ill allow it, as it shouldn't hurt anything.
  Fri Aug 26 16:16:31 2011 DOHERTY [...] cpan.org - Correspondence added
On Fri Aug 26 16:12:47 2011, exobuzz wrote: Show quoted text
> whats the problem with passing '' instead of undef?
my $result = $api->api({ thing => $value || '', otherthing => $other_value || '', verbose => $verbose || '', anything => $omg_not_DRY || '', # Are we seeing the point? });
  Fri Aug 26 16:18:26 2011 buzz [...] exotica.org.uk - Correspondence added
On Fri Aug 26 16:16:31 2011, DOHERTY wrote: Show quoted text
> On Fri Aug 26 16:12:47 2011, exobuzz wrote:
> > whats the problem with passing '' instead of undef?
> > my $result = $api->api({ > thing => $value || '', > otherthing => $other_value || '', > verbose => $verbose || '', > anything => $omg_not_DRY || '', > # Are we seeing the point? > });
why would you be passing undefined variables in? that code is no good sure, but then it shouldnt be needed. making undef work may well allow errors to get through by accident, as I would think an undefined value in a variable you are passing in wouldn't be intentional.
  Fri Aug 26 16:22:32 2011 buzz [...] exotica.org.uk - Correspondence added
Show quoted text
> why would you be passing undefined variables in? that code is no good > sure, but then it shouldnt be needed. making undef work may well allow > errors to get through by accident, as I would think an undefined value > in a variable you are passing in wouldn't be intentional.
with your same argument you could argue that when concatenating strings in perl it should automatically take undefined variables as '' also without giving a warning. im not convinced this is a good thing.
  Fri Aug 26 16:29:23 2011 DOHERTY [...] cpan.org - Correspondence added
On Fri Aug 26 16:22:32 2011, exobuzz wrote: Show quoted text
> with your same argument you could argue that when concatenating strings > in perl it should automatically take undefined variables as '' also > without giving a warning.
YES! Exactly!! When it is a good idea, you can turn off that warning in a particular scope. But I cannot reach into your module to turn off warnings there if I think they are spurious. In any event, I actually did fix the things that might be undef (or at least, the ones I found). I just think it'd be nice to not have to worry about it so much.
  Fri Aug 26 16:34:26 2011 buzz [...] exotica.org.uk - Correspondence added
Show quoted text
> YES! Exactly!! When it is a good idea, you can turn off that warning in a > particular scope. But I cannot reach into your module to turn off > warnings there if I think they are spurious. > > In any event, I actually did fix the things that might be undef (or at > least, the ones I found). I just think it'd be nice to not have to worry > about it so much.
say someone mistypes a variable name, so the module gets undef, with your change, it will not throw a warning, which might have helped them to see they were passing the wrong value. maybe i'll allow it as an option though. and then you can just switch that on.
  Fri Aug 26 16:53:05 2011 DOHERTY [...] cpan.org - Correspondence added
On Fri Aug 26 16:34:26 2011, exobuzz wrote: Show quoted text
> say someone mistypes a variable name, so the module gets undef, with > your change, it will not throw a warning
The warning you get is mostly useless, since it emits from URI::Escape, not MediaWiki::Bot, and it certainly isn't carped, so I can see where the error is in my code. As I said, I don't /really/ need this any more, I just think it'd be nice to be more lenient in what you accept.
  Fri Aug 26 16:53:28 2011 DOHERTY [...] cpan.org - Correspondence added
I meant MediaWiki::API >.<
  Sat May 19 15:09:08 2012 buzz [...] exotica.org.uk - Correspondence added
Was just looking at a few changes to the module. This issue doesn't seem to happen anymore, I assume due to changes with uri_escape_utf8 (or functions that it uses). also worth noting is that the check ($value || '') would break cases where someone passes the number 0 in. Although as the fix doesn't seem needed anymore im inclined to leave it.
  Sat May 19 18:41:59 2012 DOHERTY [...] cpan.org - Correspondence added
On Sat May 19 15:09:08 2012, exobuzz wrote: Show quoted text
> also worth noting is that the check ($value || '') would break cases > where someone passes the number 0 in.
Yes, you'll want defined-or instead.
  Sat Jan 19 11:35:20 2013 buzz [...] exotica.org.uk - Correspondence added
Closing as the issue now seems resolved.
  Sat Jan 19 11:35:21 2013 buzz [...] exotica.org.uk - Status changed from 'open' to 'resolved'