I stumbled upon a bug on row 83 in views_handler_argument_many_to_one.inc where I think that a variable should be sent by reference to work in PHP 4:

views_break_phrase($this->argument, $this);

The function views_break_phrase() doesn't takes $this as a reference and the function that calls views_break_phrase() doesn't take care of the response from the function which should result in that no values are selected when it's run on PHP 4.

CommentFileSizeAuthor
#7 329390-php-4-break-phrase.patch4.28 KBmerlinofchaos

Comments

voxpelli’s picture

voxpelli commented
Priority: Normal » Minor

Changed priority to minor - PHP 4 shouldn't be used anymore.

voxpelli’s picture

voxpelli commented

This bug also exists at row 78 in views_handler_argument_numeric.inc

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Closed (won't fix)

views_break_phrase does not need a reference.

voxpelli’s picture

voxpelli commented

I can't see how the values assigned to a non-referenced object in PHP 4 and which return value isn't taken care of can be available in the calling function.

Can you describe how that can be possible? In 99% of all cases it works great - but at the two places I have found it shouldn't be working as far as I know.

merlinofchaos’s picture

merlinofchaos commented

It does seem that the return value needs to be dealt with, but you can't pass the value by reference there.

voxpelli’s picture

voxpelli commented
Title: Potential PHP 4 bug: Variable should be sent as reference » Potential PHP 4 bug: Variable missing reference
Status: Closed (won't fix) » Active

Ok, I change the Issue title thenand reactivate this issue - because I agree with you that the return values should be used in views_handler_argument_many_to_one.inc and views_handler_argument_numeric.inc.

It's not me that isn't using the return values - it's views itself that calls views_break_phrase without using it's return values because the $this object is sent as a reference in PHP 5 so the bug would only appear in PHP 4 as I see it.

merlinofchaos’s picture

merlinofchaos commented
Version: 6.x-2.1 » 6.x-2.x-dev
Status: Active » Needs review
StatusFileSize
new 329390-php-4-break-phrase.patch4.28 KB

As near as I can tell, every instance of views_break_phrase in use actually has the second argument, so it does not need to be optional. We might want to scan contrib and see if anyone is using it, but let's throw this up for testing:

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Fixed

Committed to 6.x branches. No point in committing to 7.x as PHP4 isn't good enough for Drupal 7.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.