CodeIgniter Forums
Please consider querystring support in 2.x - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Archived Discussions (https://forum.codeigniter.com/forumdisplay.php?fid=20)
+--- Forum: Archived General Discussion (https://forum.codeigniter.com/forumdisplay.php?fid=21)
+--- Thread: Please consider querystring support in 2.x (/showthread.php?tid=31698)

Pages: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]WanWizard[/eluser]
Yes, I did. But before I could finish my reply to that I had to go into a meeting. Have to make some money every now and then... Wink

[quote author="skunkbad" date="1280490971"]1) Once we are getting the value of allow_get_array from our new config variable, why do we care if there is a enable_query_strings variable in the Input class? It doesn't seem to get used anywhere in the class. Are other classes checking this value here instead of getting it from the config class?
2) What is the reason why the Input class needs to load between Config and URI?[/quote]
Good questions.

I just ran an svn diff and posted the results here, might be that some leftovers from earlier changes made it to the diff.

I'll review the changes, you could probably get away with simply changing
Code:
$this->allow_get_array = ($CFG->item('enable_query_strings') === TRUE) ? TRUE : FALSE;
to
Code:
$this->allow_get_array = ($CFG->item('allow_get_array') === TRUE) ? TRUE : FALSE;
and adding that config value to your config file.


EDIT: no, you need the modification of URI.php too, to avoid using $_GET when the uri_protocol is AUTO:
Code:
if ($this->config->item('enable_query_strings') === TRUE && is_array($_GET) && count($_GET) == 1 && trim(key($_GET), '/') != '')

EDIT2: I now realise where the diff entries come from. I first modified the meaning of the existing config key 'enable_query_string', which required modifications in codeigniter.php and router.php as well. I decided later that it wasn't handy, and created the new 'allow_get_array' config key, which is a much simpler change. Unfortunately, I generated the diff using the wrong repo revisions as reference, which severly messed things up.

Thanks for pointing this out, and sorry to all of you, for making this sound more complicated than it was.

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]skunkbad[/eluser]
[quote author="WanWizard" date="1280501676"]...you could probably get away with simply changing
Code:
$this->allow_get_array = ($CFG->item('enable_query_strings') === TRUE) ? TRUE : FALSE;
to
Code:
$this->allow_get_array = ($CFG->item('allow_get_array') === TRUE) ? TRUE : FALSE;
and adding that config value to your config file.


EDIT: no, you need the modification of URI.php too, to avoid using $_GET when the uri_protocol is AUTO:
Code:
if ($this->config->item('enable_query_strings') === TRUE && is_array($_GET) && count($_GET) == 1 && trim(key($_GET), '/') != '')

EDIT2: I now realise where the diff entries come from. I first modified the meaning of the existing config key 'enable_query_string', which required modifications in codeigniter.php and router.php as well. I decided later that it wasn't handy, and created the new 'allow_get_array' config key, which is a much simpler change. Unfortunately, I generated the diff using the wrong repo revisions as reference, which severly messed things up.

Thanks for pointing this out, and sorry to all of you, for making this sound more complicated than it was.[/quote]

So really, with these two changes, we have a reliable way to use $_GET. I haven't tested, but the kosher way of making these changes would be in a MY_Input and MY_Uri. It seems odd that CI doesn't have this already. From working without $_GET for so long, I can do most anything without it, but it's nice to know we have a reliable solution if we need it, and that the changes needed are so simple to implement.

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]skunkbad[/eluser]
This is working, and probably the kosher way:

MY_Input.php (as suggested a few posts back)
Code:
<?php
// this allows for $_GET access
class MY_Input extends CI_input {
    function _sanitize_globals() {
        $this->allow_get_array = TRUE;
        parent::_sanitize_globals();
    }
}

MY_URI.php
Code:
<?php
// this allows for $_GET access and uri segments to work together
class MY_URI extends CI_URI{
    function _fetch_uri_string()
    {
        if (strtoupper($this->config->item('uri_protocol')) == 'AUTO')
        {
            // If the URL has a question mark then it's simplest to just
            // build the URI string from the zero index of the $_GET array.
            // This avoids having to deal with $_SERVER variables, which
            // can be unreliable in some environments
            if ($this->config->item('enable_query_strings') === TRUE && is_array($_GET) && count($_GET) == 1 && trim(key($_GET), '/') != '')
            {
                $this->uri_string = key($_GET);
                return;
            }

            // Is there a PATH_INFO variable?
            // Note: some servers seem to have trouble with getenv() so we'll test it two ways
            $path = (isset($_SERVER['PATH_INFO'])) ? $_SERVER['PATH_INFO'] : @getenv('PATH_INFO');
            if (trim($path, '/') != '' && $path != "/".SELF)
            {
                $this->uri_string = $path;
                return;
            }

            // No PATH_INFO?... What about QUERY_STRING?
            $path =  (isset($_SERVER['QUERY_STRING'])) ? $_SERVER['QUERY_STRING'] : @getenv('QUERY_STRING');
            if (trim($path, '/') != '')
            {
                $this->uri_string = $path;
                return;
            }

            // No QUERY_STRING?... Maybe the ORIG_PATH_INFO variable exists?
            $path = str_replace($_SERVER['SCRIPT_NAME'], '', (isset($_SERVER['ORIG_PATH_INFO'])) ? $_SERVER['ORIG_PATH_INFO'] : @getenv('ORIG_PATH_INFO'));
            if (trim($path, '/') != '' && $path != "/".SELF)
            {
                // remove path and script information so we have good URI data
                $this->uri_string = $path;
                return;
            }

            // We've exhausted all our options...
            $this->uri_string = '';
        }
        else
        {
            $uri = strtoupper($this->config->item('uri_protocol'));

            if ($uri == 'REQUEST_URI')
            {
                $this->uri_string = $this->_parse_request_uri();
                return;
            }

            $this->uri_string = (isset($_SERVER[$uri])) ? $_SERVER[$uri] : @getenv($uri);
        }

        // If the URI contains only a slash we'll kill it
        if ($this->uri_string == '/')
        {
            $this->uri_string = '';
        }
    }
}


Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]WanWizard[/eluser]
Probably.

But I have a big issue with all these CI extensions. By the time my controller is ready to start about its business and generate some output, I have almost 15Mb of memory used loading code that isn't used (since the functions are overloaded with my extensions).

So I've forked CI 2.0, and I'm slowly migrating my changes from my extensions into the core code. And leave the extensions only for new methods.

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]DJMOHNL[/eluser]
[quote author="pbreit" date="1280481955"]Coder, is that a complete solution? Any side effects?[/quote]
yes its complete solution, you cant use http://yoursite.com/index.php/ anymore but who wants that ?

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]skunkbad[/eluser]
WanWizard, I understand. So, at least we have some choices, and people that are looking to use $_GET can relax because this thread exists. Now if something could just be worked into CI 2.x, the world would be a better place.

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]WanWizard[/eluser]
[quote author="DJMOHNL" date="1280548289"]yes its complete solution, you cant use http://yoursite.com/index.php/ anymore but who wants that ?[/quote]
What do you exactly mean by this?

If it breaks standard CI behaviour in some way, it's not a complete solution. Can you tell me what no longer works, so I can have a look and try to come up with a better solution?

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]DJMOHNL[/eluser]
[quote author="skunkbad" date="1280529093"]This is working, and probably the kosher way:

MY_Input.php (as suggested a few posts back)
Code:
<?php
// this allows for $_GET access
class MY_Input extends CI_input {
    function _sanitize_globals() {
        $this->allow_get_array = TRUE;
        parent::_sanitize_globals();
    }
}

MY_URI.php
Code:
<?php
// this allows for $_GET access and uri segments to work together
class MY_URI extends CI_URI{
    function _fetch_uri_string()
    {
        if (strtoupper($this->config->item('uri_protocol')) == 'AUTO')
        {
            // If the URL has a question mark then it's simplest to just
            // build the URI string from the zero index of the $_GET array.
            // This avoids having to deal with $_SERVER variables, which
            // can be unreliable in some environments
            if ($this->config->item('enable_query_strings') === TRUE && is_array($_GET) && count($_GET) == 1 && trim(key($_GET), '/') != '')
            {
                $this->uri_string = key($_GET);
                return;
            }

            // Is there a PATH_INFO variable?
            // Note: some servers seem to have trouble with getenv() so we'll test it two ways
            $path = (isset($_SERVER['PATH_INFO'])) ? $_SERVER['PATH_INFO'] : @getenv('PATH_INFO');
            if (trim($path, '/') != '' && $path != "/".SELF)
            {
                $this->uri_string = $path;
                return;
            }

            // No PATH_INFO?... What about QUERY_STRING?
            $path =  (isset($_SERVER['QUERY_STRING'])) ? $_SERVER['QUERY_STRING'] : @getenv('QUERY_STRING');
            if (trim($path, '/') != '')
            {
                $this->uri_string = $path;
                return;
            }

            // No QUERY_STRING?... Maybe the ORIG_PATH_INFO variable exists?
            $path = str_replace($_SERVER['SCRIPT_NAME'], '', (isset($_SERVER['ORIG_PATH_INFO'])) ? $_SERVER['ORIG_PATH_INFO'] : @getenv('ORIG_PATH_INFO'));
            if (trim($path, '/') != '' && $path != "/".SELF)
            {
                // remove path and script information so we have good URI data
                $this->uri_string = $path;
                return;
            }

            // We've exhausted all our options...
            $this->uri_string = '';
        }
        else
        {
            $uri = strtoupper($this->config->item('uri_protocol'));

            if ($uri == 'REQUEST_URI')
            {
                $this->uri_string = $this->_parse_request_uri();
                return;
            }

            $this->uri_string = (isset($_SERVER[$uri])) ? $_SERVER[$uri] : @getenv($uri);
        }

        // If the URI contains only a slash we'll kill it
        if ($this->uri_string == '/')
        {
            $this->uri_string = '';
        }
    }
}
[/quote]

next time you try create an MY_Input.php youll see that you need to add all the functions inside this one, wich isnt very smart to do.. Yust add the system "get patch" and it gives you a far better patch (nothing gets broken, its tested by more than a dosen people here)

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]DJMOHNL[/eluser]
[quote author="skunkbad" date="1280467167"]DJMOHNL, you probably ought to set $config['allow_get_array'] to TRUE by default.[/quote]

thanks, as i just copied it from WanWizzard with the param "FALSE" wicht wont enven work.Tongue
but its now TRUE inside the file updated..Wink

Please consider querystring support in 2.x - El Forum - 07-30-2010

[eluser]skunkbad[/eluser]
[quote author="DJMOHNL" date="1280561717"]next time you try create an MY_Input.php youll see that you need to add all the functions inside this one, wich isnt very smart to do.. Yust add the system "get patch" and it gives you a far better patch (nothing gets broken, its tested by more than a dosen people here)[/quote]

I know that patching the system libraries works, because I tried it too. I was simply providing a solution that shows extending the classes "the CI way", because this is the way most people should do it. If they ever upgrade their installation to a new version of CI, they won't wipe out a custom functionality of their site that they "patched" or "hacked".

Heck, I don't even use $_GET. I'm just here trying to help solve an ongoing complaint about CI's $_GET array deficiency. If you needed to have other extended functionality of the Input class, yes you would have to change MY_Input to suit your needs. I am smart enough to do it.

In the case of WanWizard and his complaint about extending classes and performance issues, this is a legitimate complaint. Your hacking of core CI classes, and not extending those classes because you might fumble the next time they require modification is just a lame excuse.

In what way does your beloved patched method provide a far better patch? It's result is the same as what I have done by extending the classes, but I haven't changed core files. Prove me wrong...