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

Old() - Seems to have an issue with retrieving array values #1492

Closed
daljit3 opened this issue Nov 16, 2018 · 11 comments
Closed

Old() - Seems to have an issue with retrieving array values #1492

daljit3 opened this issue Nov 16, 2018 · 11 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them in progress

Comments

@daljit3
Copy link

daljit3 commented Nov 16, 2018


name: Bug report
about: Help us improve the framework by reporting bugs!


Describe the bug
My input field is named as "locations[]". When the form failed validation - I tried to retrieve it as $locations = old('locations'); Which throws up an error - strpos() expects parameter 1 to be string, array given - BASEPATH/Common.php at line 786

The line 786 is in old() method in System/Common.php is
// If the result was serialized array or string, then unserialize it for use... if (strpos($value, 'a:') === 0 || strpos($value, 's:') === 0) { $value = unserialize($value); }
Here are the full methods from all the files which I think are related to this issue.
You can see the $value is being parsed through $request->getOldInput($key); in this old() method. There isn't any where I can see the arrays in POST data being serialised as I looked into WithInput() method too.

	function old(string $key, $default = null, $escape = 'html')
	{
		$request = Services::request();

		$value = $request->getOldInput($key);

		// Return the default value if nothing
		// found in the old input.
		if (is_null($value))
		{
			return $default;
		}

		// If the result was serialized array or string, then unserialize it for use...
		if (strpos($value, 'a:') === 0 || strpos($value, 's:') === 0)
		{
			$value = unserialize($value);
		}

		return $escape === false ? $value : esc($value, $escape);
	}

** File system/HTTP/IncomingRequest.php **

	public function getOldInput(string $key)
	{
		// If the session hasn't been started, or no
		// data was previously saved, we're done.
		if (empty($_SESSION['_ci_old_input']))
		{
			return;
		}

		// Check for the value in the POST array first.
		if (isset($_SESSION['_ci_old_input']['post'][$key]))
		{
			return $_SESSION['_ci_old_input']['post'][$key];
		}

		// Next check in the GET array.
		if (isset($_SESSION['_ci_old_input']['get'][$key]))
		{
			return $_SESSION['_ci_old_input']['get'][$key];
		}

		helper('array');

		// Check for an array value in POST.
		if (isset($_SESSION['_ci_old_input']['post']))
		{
			$value = dot_array_search($key, $_SESSION['_ci_old_input']['post']);
			if (! is_null($value))
			{
				return $value;
			}
		}

		// Check for an array value in GET.
		if (isset($_SESSION['_ci_old_input']['get']))
		{
			$value = dot_array_search($key, $_SESSION['_ci_old_input']['get']);
			if (! is_null($value))
			{
				return $value;
			}
		}
	}

** File System/HTTP/RedirectResponse.php **

	public function withInput()
	{
		$session = $this->ensureSession();

		$input = [
			'get'  => $_GET ?? [],
			'post' => $_POST ?? [],
		];

		$session->setFlashdata('_ci_old_input', $input);

		// If the validator has any errors, transmit those back
		// so they can be displayed when the validation is
		// handled within a method different than displaying the form.
		$validator = Services::validation();
		if (! empty($validator->getErrors()))
		{
			$session->setFlashdata('_ci_validation_errors', serialize($validator->getErrors()));
		}

		return $this;
	}

CodeIgniter 4 version
4.0.0-alpha.2 Released

Affected module(s)
System/Common.php, System/HTTP/IncomingRequest.php, System/HTTP/RedirectResponse.php

Expected behavior, and steps to reproduce if appropriate
It should return full array.

@daljit3 daljit3 changed the title Old() - Seems to have issue with retrieving array values Old() - Seems to have an issue with retrieving array values Nov 16, 2018
@jim-parry jim-parry added bug Verified issues on the current code behavior or pull requests that will fix them in progress labels Nov 18, 2018
@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

I just tried "git pull" to pull latest changes and I can see that it pulled in System/Common.php and System/HTTP/IncomingRequest.php plus some test files - however, I am unsure but it seemed to have an affect on redirect(). Any calls to redirect just end up with a blank page?

@jim-parry
Copy link
Contributor

@daljit3 Can you provide an example of a redirect that ends up with a blank page?
All the unit tests pass, so this is not something I would expect.

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

@jim-parry Thanks for getting back to me quickly. Please take a look at this method below. Yesterday, all the code was working fine until I found the bug above. Today, I saw your email that it has been fixed. I did git pull so that I can check if it was fixed. I tried to login to my site on my local but it gave me a blank page after submitting my login credentials. I manually changed the url to home page and I could see that it did log me in. I tried to logout where all I have is the code below and it just gives me a blank page without redirecting.

    public function logout()
    {
        session()->destroy();

        setFlashAlertSuccess('You have logged out successfully!');
        redirect()->to('/');
        //redirect("/");
        exit;
    }

@lonnieezell
Copy link
Member

When doing redirects, you should always return the redirect, like so:

return redirect()->to('/');

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

I also tried return redirect()->to("/"); as well but no luck.

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

@lonnieezell yes when I downloaded the very first release of CI4, I did have a return on all of my redirects - but unfortunately that never seemed to have worked for me. That's why I ended up having to use exit; after redirects through out my site. I just tried once again with return but it just shows me a blank page. I have up to date code as I use a cloned copy of your CI4 from Github on local and I do git pull everyday.

@lonnieezell
Copy link
Member

@daljit3 Gotcha. It was designed so that you have to do a return, but sounds like there's an issue there. I know they've made some changes to redirects in the last couple of days trying to clean things up, and I've been slammed with the day job for the past bit so I'm not 100% in the loop on the changes here. I'll let @jim-parry continue on with this one.

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

Yes - I can confirm there is an issue with redirects. I just downloaded a new copy of CI4-develop and I could see welcome message to confirm everything working fine. Then I made these modifications below to the Home controller. If I try mylocalsite/home/daljit it just shows a blank page.

 namespace App\Controllers;

use CodeIgniter\Controller;

class Home extends Controller
{
	public function index()
	{
		return view('welcome_message');
	}

	//--------------------------------------------------------------------
    public function daljit()
    {
        return redirect()->to("/home/hello");
    }

    public function hello()
    {
        echo "Hello!";
    }
}

@jim-parry jim-parry reopened this Nov 18, 2018
@jim-parry
Copy link
Contributor

I will revert the changes I made to "fix old()" and start from there.

@jim-parry
Copy link
Contributor

Done, and I will comment further on the other bug report.

@jim-parry
Copy link
Contributor

Solution confirmed, phew

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them in progress
Projects
None yet
Development

No branches or pull requests

3 participants