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.

Sign up for GitHub

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

Limit storePreviousURL to certain requests #2126

Merged
merged 4 commits into from
Aug 15, 2019
Merged

Limit storePreviousURL to certain requests #2126

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eccbfc0
Limit storePreviousURL to certain requests
MGatner Aug 11, 2019
70f1e57
Remove IncomingRequest dependency
MGatner Aug 12, 2019
8d86edb
Switch to use is_cli()
MGatner Aug 13, 2019
0669f8d
Merge remote-tracking branch 'upstream/develop' into ajax-return-url
MGatner Aug 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions system/CodeIgniter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
use Closure;
use CodeIgniter\Filters\Exceptions\FilterException;
use CodeIgniter\HTTP\DownloadResponse;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\RedirectResponse;
use CodeIgniter\HTTP\Request;
use CodeIgniter\HTTP\ResponseInterface;
Expand Down Expand Up @@ -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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

{
return;
}

// This is mainly needed during testing...
if (is_string($uri))
{
Expand Down