[wp-hackers] Getting Bugs Noticed

[Brian Krausz]

Sorry for the slow reply:

Since I'm the one to write the patch which contributed the most to
> _wp_filter_build_unique_id(), I'll say that you are doing it wrong. The
> function was never meant to handle your use case. It is working as it
> supposed to. The issue is that you are changing the data of what the
> function sees as the same instance (it isn't, which is where it appears the
> bug stems from).


Exactly my point, two instances should not be considered the "same", since
every instance of a class can have its own branching.  You can't assume a
class will always add the same action as their first add_action call.


> If you work it out, the first instance works out to Foobaz0 and the second
> instance should work out to Foobaz1. This is assuming that the plugin code
> is isolated. I will have to take a further look at it, because it does not
> seem like bar() is doing anything in your example, but the result should be
> "12" and not just "2". So most likely your use case was not tested fully.
> That is easy enough to correct, just write test cases for your example and
> see what happens and if it fails, then attempt to fix it until the tests
> pass.


That's more or less what I did, but with my actual code: the exact fix was
swapping the init call and the other call.


> Where you are incorrect is that once array(&$this, 'bar') is added to
> 'something1', it should gain the _wp_filter_id to 0 and when array(&$this,
> 'baz') is added to 'init' hook, the _wp_filter_id should already exist. You
> are correct in that when the second instance is created and it goes to add
> array(&$this, 'bar') to 'something2' it will not find _wp_filter_id, nor
> will it find any other items in 'something2', so it will also set it to 0.
> You are correct in that instead of getting the correct 'Foobaz0' and
> 'Foobaz1', you end up with both 'Foobaz0'. Interestingly, if you had
> something actually echoed in 'bar', you'll probably see both items (in
> theory). In the case of the 'init' hook, the second instance is replacing
> the first, which is why you only see '2'.


> This could be fixed if you had the 'init' hook first instead of second. It
> is probably why the static method worked in your case, because for static
> methods, the _wp_filter_id is not set, therefore when it gets to the init
> hook, it sets it and then counts the amount of the hooks. It seems like your
> bug is probably biting the asses of all of the dumbasses who are writing
> their plugins incorrectly. Good chance!


I understand the reasoning here, and know the solution to the situation, but
IMHO the function doesn't follow its spec.  The spec (though not explicitly
stated), is to hash an object so that it can be stored without risk of
overwriting instances of other objects.  Since there exists a way to cause
objects to overwrite each other while still following the spec of
add_action, I see a problem.


> Unfortunately, I do not see a fix at this time. If you can think of another
> algorithm to make the function more idiot proof, it should suffice until the
> world creates a bigger idiot (not you, you found the problem and reported
> it, I'm talking about those who are affected by this and don't know it).


What about using a static var that's incremented with each read to fill
wp_filter_id.  That guarantees it will be unique amongst other objects.  It
could track class names so the id stays smaller, but I see no benefit to
that...


> Jacob Santos
>
> PS: It will it go a long way to getting things solved, if you create a
> patch for a possible solution. I could really care less for #8731, therefore
> someone else is going to need to look at it or you are going to have to
> handle it yourself. Now, I might have some interest in it if you slap me
> around a little and threaten to harm my next door neighbor who I could also
> care less about.


Thanks for the advice, I didn't realize patches push things along that
well.  I'm away vacation soon, so my responses may be delayed, but my patch
is attached to the bug.

Thanks,
Brian

Brian Krausz wrote:
>
>>  Hey guys,
>>
>>  I have two bugs from a while back that seem to have gone ignored:
>>
>>  https://core.trac.wordpress.org/ticket/8723
>>  https://core.trac.wordpress.org/ticket/8731
>>
>>  I'm wondering if there's something I'm doing that causes these bugs
>>  to go unnoticed?  I feel bad assigning them to random people, but I
>>  would like them to be discussed, and they need a consensus before I
>>  can submit a patch for them.
>>
>>  Thanks, Brian _______________________________________________
>>  wp-hackers mailing list wp-hackers at lists.automattic.com
>>  http://lists.automattic.com/mailman/listinfo/wp-hackers
>>
>
>
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
>