Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent undefined array key access in FormInput helper #1040

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

csidirop
Copy link
Contributor

Hi, I wrote some plugins the past days and used the formInput() function in my config forms. All occurrences throwing following warning:
[php:warn] [pid 69748] [client 172.24.0.1:55254] PHP Warning: Undefined array key "type" in /app/application/views/helpers/FormInput.php on line 42, referer: http://localhost:8202/nikephoros/admin/plugins

If we call formInput() without an attribute parameter it defaults it to null. But later in L42 we accessing that attribute without checking if its set:

$type = 'text';
if ($attribs['type']) {
$type = $attribs['type'];
unset($attribs['type']);
}

This PR adds a simple 'isset()' check, so we do not access a unset variable.

@csidirop
Copy link
Contributor Author

In addition here is one example how I used it:

<?php
    $option = get_option('option');
?>

<div class="field">
    <div class="two columns alpha">
        <?php echo get_view()->formLabel('option', __('Option Name')); ?>
    </div>
    <div class="inputs five columns omega">
        <p class="explanation">
            <?php echo __('The Desc.'); ?>
        </p>
        <?php echo get_view()->formInput('option', $option); ?>
    </div>
</div>

I didn't found any other plugin which used this, so I could not verify how others used it. So added my code for reference.

@zerocrates
Copy link
Member

zerocrates commented Nov 27, 2024

The reason this isn't accounted for, and you're not seeing other examples to go from, is that formInput is designed for cases where you want to use one of the "new" input types, color, date, etc. Just a regular text input like this, the normal way to do that is to use formText. In other words, the use case for formInput is that you'd always be passing type.

Of course I do agree it shouldn't be a notice anyway.

@csidirop
Copy link
Contributor Author

csidirop commented Nov 28, 2024

Okay I see, thats why I only found formText. So I have to change my code. Thanks!

Than we should update the example in the docs where it uses it like I had <?php echo get_view()->formInput('some-element', $someElementValue); ?>:
https://github.com/omeka/Documentation/blob/master/source/Tutorials/bestPracticesPlugins.rst?plain=1#L283-L295

Nevertheless the issue is still valid.

@zerocrates zerocrates merged commit 09d7e39 into omeka:master Dec 3, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants