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

Input: considering nested array keys in get_post and post_get #5531

Merged
merged 3 commits into from Jul 22, 2018

Conversation

tianhe1986
Copy link
Contributor

With URL such as http://ci.mine.cn/test/test_get?a[b]=test and the following code

$something = $this->input->get('a[b]');
$something2 = $this->input->get_post('a[b]');
var_dump($something, $something2);

$something gets 'test', and $something2 turns out to be NULL. However, they are expected to be the same.

So I try to fix it using null comparison instead of isset check.

Signed-off-by: tianhe1986 <w1s2j3229@163.com>
Signed-off-by: tianhe1986 <w1s2j3229@163.com>
@narfbg
Copy link
Contributor

narfbg commented Jun 23, 2018

An isset check is a null comparison (that's not where the difference in results come from), and I insist that isset() still be used.

@tianhe1986
Copy link
Contributor Author

@narfbg No, it is just isset causing the difference, because in get() it doing more check than just null comparison.
With the URL I showed above, isset($_GET['a[b]']) would return false, resulting in get_post fallback to post() and get NULL at last.
It should fallback to array notation check with $_GET first, and then fallback to post() when still getting NULL.

@narfbg
Copy link
Contributor

narfbg commented Jun 24, 2018

No ... You don't get it.

isset() is literally a "defined and not null" check; effectively a fast @$var !== NULL check - that's not the issue at all. The reason why isset($_GET['a[b]']) will return false is because that should be isset($_GET['a']['b']), but of course you can't do that dynamically.

The difference, both visible in your patch and as you said it, is that get() (well, _fetch_from_array()) does more work, in that it matches the string, pseudo-key input against the actual array keys.

When I say I insist on isset() being used, I mean isset($output) as the ternary condition. Although, now I take that back ... we don't need that variable at all, the following should work just fine:

return $this->get($index, $xss_clean) ?: $this->post($index, $xss_clean);

@tianhe1986
Copy link
Contributor Author

I see. I'm sorry for misunderstanding what you said.
However, ?: is not right for variables such as 0, '0' and empty string for that it is a TRUE check, so isset() should also be used.

$output = $this->post($index, $xss_clean);
return isset($output) ? $output : $this->get($index, $xss_clean);

Is this OK?

@narfbg
Copy link
Contributor

narfbg commented Jun 25, 2018

You're right, I always mix up short ternary with null coalesce. :)

Yes, that'd be exactly what I wanted.

Signed-off-by: tianhe1986 <w1s2j3229@163.com>
@narfbg narfbg added this to the 3.2.0 milestone Jul 22, 2018
@narfbg narfbg merged commit 29b2ac0 into bcit-ci:develop Jul 22, 2018
narfbg added a commit that referenced this pull request Jul 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants