Limit storePreviousURL to certain requests #2126
Conversation
system/CodeIgniter.php
Outdated
| @@ -968,6 +969,12 @@ protected function gatherOutput($cacheConfig = null, $returned = null) | |||
| */ | |||
| public function storePreviousURL($uri) | |||
| { | |||
| // Only valid for some requests | |||
| if (! $this->request instanceof IncomingRequest || $this->request->isCLI() || $this->request->isAJAX()) | |||
There was a problem hiding this comment.
I'm a little concerned with checking against IncomingRequest. it's entirely possible for someone to use a completely different IncomingRequest-like class. While I would think they would extend IncomingRequest there's nothing that limits them from doing something crazy. Better to either not check against a class type and just do the AJAX and CLI checks, or check against Request.
There was a problem hiding this comment.
That is how I had it originally but since neither Request nor RequestInterface require isAJAX() and isCLI() it was causing issues, e.g. CLI requests where giving "method doesn't exist" exceptions.
I guess I will try it by checking for each method, but I'm also open to an additional interface or some other means of defining a "browser visit".
system/CodeIgniter.php
Outdated
| // Only valid for some requests | ||
| if (! $this->request instanceof IncomingRequest || $this->request->isCLI() || $this->request->isAJAX()) | ||
| // Ignore CLI requests | ||
| if (method_exists($this->request, 'isCLI') && $this->request->isCLI()) |
There was a problem hiding this comment.
is_cli() is an available function provided in Common.php, so no need to check, just use the helper function.
Unfortunately, none exists for is_ajax.
|
You had a productive night! I’ll make that change. Would you like me to take a crack at |
|
Feel free to take a look at it. I think that it requires info from IncomingRequest, though. |
|
Okay I'll look at that later - we'll pick the low-hanging fruit for now. |
|
@lonnieezell This one is all set. |
|
is_ajax would be a IncomingRequest because it gets a lot of the information from $_SERVER[] |
Description
Currently
storePreviousUrl()will fire on AJAX requests, saving the URL from the AJAX request and causingredirect()->back()to return to routes that probably were never intended for direct browsing.This change causes
$_SESSIONto be updated only when it is a validIncomingRequestthat is notCLIorAJAX(including these additional parameters helps with testing).Checklist: