1

Code used for processing form $_POST submissions is working well on most forms but suddenly broke on a new set of forms. I can't see any difference in the forms themselves as it's based purely on the posted values and I have it fixed but I am curious why the sudden problem.

There are some cases where specific post values are not to be processed and those, when they are not needed, are in $RemoveFields as a comma-separated list which is converted to an array and on the one set of forms, it doesn't matter if $RemoveFields has any value or not but on the other set it crashes when empty.

By adding a conditional I was able to make it work but can anyone tell me what the problem is on the original code? Both the old and new are below. The first works on only some of the forms while the second seems to work on all.

The original code:

// Remove unneeded fields specified in $RemoveFields variable
if (isset($RemoveFields) && !is_array($RemoveFields)) $RemoveFields = array($RemoveFields);
$filteredarray = array_diff_key($_POST, array_flip($RemoveFields));

The same code but with a conditional for the $filteredarray value:

// Remove unneeded fields specified in $RemoveFields variable
if (isset($RemoveFields) && !is_array($RemoveFields)) $RemoveFields = array($RemoveFields);
$filteredarray = (isset($RemoveFields)) ? array_diff_key($_POST, array_flip($RemoveFields)) : $_POST;
DonP
  • 725
  • 1
  • 8
  • 27
  • 2
    If `$RemoveFields` is a comma-separated value, shouldn't you use `explode()` to turn it into an array? – Barmar Jan 26 '19 at 00:57
  • 2
    If `$RemoveFields` is not set, the second line will try to do `array_flip(null)`, which is invalid. – Barmar Jan 26 '19 at 00:59
  • @Barmar This was only a snippet of a much, much longer function and possibly simplified here a bit too much but the description indicated that $RemoveFields is ultimately an array if it has any value at all. As for array_flip(null) not working, that makes sense which is why the question as some forms where $RemoveFields has no value, which is most of them, it still works. Can it be that array_flip is getting something other than null, such as 0 and if that's the case, would it still throw errors? – DonP Jan 26 '19 at 07:48

1 Answers1

1

In the original code, you call array_flip($RemoveFields) even when $RemoveFields is not set. This fails because the argument to array_flip() must be an array.

You should use isset() to protect both lines of code:

if (isset($RemoveFields)) {
    if (!is_array($RemoveFields)) {
        $RemoveFields = array($RemoveFields);
    }
    $filteredarray = array_diff_key($_POST, array_flip($RemoveFields));
} else {
    $filteredarray = $_POST;
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Yes, that's essentially what I showed in the second version but the main question is why the original code works with no errors when $RemoveFields has no value, which is most of the forms. The question wouldn't have even come up if not for a set of new forms that throw errors when none of the original forms do. I should clarify that these forms are being created in an unconventional way with the first being generated using an array of values and the second (the ones with the problem) being generated by an identical array but with its source from a table in the database. – DonP Jan 26 '19 at 07:55
  • $RemoveFields is always set but rarely has any value other than in the few places where it's needed. No matter, I think I have the general idea of why this suddenly failed and that was the point of the original question. Thank you. – DonP Jan 26 '19 at 08:01
  • I'd have to see how you're setting (or not setting) `$RemoveFields`. Note that a variable is considered set if it's set to an empty string. If this comes from a text field in a form, it will always be set. – Barmar Jan 28 '19 at 08:57
  • In one set of forms, it is a value on the form page; in the other set, it is a value in a database column. Anyway, it’s working now. – DonP Jan 29 '19 at 09:08
  • If it's a form field, it will always be set. If the user leaves the field blank, it's set to an empty string. The only types of form fields that might not be set are checkboxes (the checked boxes are set, the unchecked boxes are not) and submit buttons (if the form has multiple buttons, only the button that was used to submit it is set). – Barmar Jan 29 '19 at 17:06
  • 1
    You might consider using `if (!empty($RemoveFields))` rather than `if(isset($RemoveFields))`. This will be false for an empty string or array as well, not just an unset variable. – Barmar Jan 29 '19 at 17:07
  • Yes, thank you, I did that after the last round of comments. – DonP Feb 01 '19 at 03:13