Closed burak closed 8 years ago
Can you give me some context? Generally, I don't feel the need to be defensive against gross violations of encapsulation. That usually falls under the guideline of "if you do that, you get to keep both pieces".
Well, in a perfect world you are absolutelyright, but I think that if you are using a cache yourself, you need to verify that to a certain level. I agree that the user code such as the thing triggered this issue is not the most beautiful code, but it happened. Having god objects and then playing with the keys if that's a hash is a high possibility for the code in the wild. With this patch you are also giving a clear pointer on what went wrong with it (and I'd like to repeat the need for the cache validation part in any context).
So, I've figured out this quickly myself but my fellow devs were confused for a (very) long while and couldn't locate the source of the error (one even claimed that my fix was irrelevant to the issue until I've explained it in detail).
Context is some code doing:
$self->{http_tiny} = HTTP::Tiny->new;
....
$obj = $self;
sub some_hairy_method {
$r = $obj->request( ... );
...
Some::Other::Thing::bla(
foo => 42,
...
ua => $obj->http_tiny,
);
# now that we have ->{handle} autovivified to a non-onject
return; # after this point something else makes use of the http_tiny key
}
sub Some::Other::Thing::bla {
... some stuff
# $ua->{handle} is undef
my $foo = $ua->{handle}{$key} // 'unknown';
...
return;
}
I appreciate that you did some work to put together a pull request, but I'm going to close this without applying it. People playing with internals of objects are implicitly agreeing to live with the consequences -- including mysterious, hard to diagnose bugs.
Alright, well you are ignoring tha fact that not everyone programming in Perl cares much about that, especially if they are coming from other backgrounds, or will bother to report issues. But in any case the issue on my end was fixed already anyways.
Yes, this happened: $ perl -MHTTP::Tiny -MData::Dumper -wE 'my $h = HTTP::Tiny->new; $h->{handle} = undef; $h->{handle}{foo}{bar} = 42; print +Dumper $h->request(GET => "http://localhost")' | grep "'content'" 'content' => 'Can\'t call method "can_reuse" on unblessed reference at /Users/burak/.plenv/versions/5.24.0/lib/perl5/site_perl/5.24.0/HTTP/Tiny.pm line 605.