Post by JNeuhoff » Thu Jul 03, 2014 6:12 pm

dfumagalli wrote: Since I know OE mods factory.php, then it's why I am asking you if you got any idea.

Thanks in advance
Send me a PM with your FTP and OpenCart login details, and an example of where it fails, and I can take a closer look at it.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Thu Jul 03, 2014 6:52 pm

Thank you so much, I wish *paid* support was as fast as you are. :clap: :clap: :clap:

I don't think it "fails", it's just doing what's it's programmed to do, but in my opinion it's extremely nasty for end users. They get lost at "blank pages with cryptic error messages" and ultimately leave the shopping.

Basically, let's see if I understand the domain of the issue:

- OC comes with a functional, own 404 page.

- OC may enable SEO URLs.

- OC SEO URLs and own 404 don't cohexist and a blank page with ugly error message is shown.

I don't know whether it's by OC wanted design or by OC "bad" design or if it's an OE introduced issue.


Where it fails it's easy to see with a debugger.


Let's start from the main index.php. I am passing a demonstration, not existant URL: http://www.domain.tld/99-bears .
Towards the end of the file there's some code that implements what I think it's one of the least known (yet awesome) OC features: the ability to "push" pre-actions and queue actions.
2014-07-03 11_22_42-Index.png

2014-07-03 11_22_42-Index.png (41.91 KiB) Viewed 5329 times

First a pre-action implementing the SEO URL controller is added (1), then the maintenance page (2) and finally the "real" page we want is scheduled to be processed by specifying it in the action (3).

What I'd expect is that the next line, the dispatch, would process the pre-actions, then the action and if the page is not found, it'd calle the specified second parameter (error/not_found.php).


This easily happens when OC has no SEO URL enabled, but when SEO URLs are enabled it implies the URL omitted the "route=" parameter and takes the bad SEO URL as it was a controller instead of just a missing page.


In fact, going ahead with the debugging, we go to factory.php's newAction and see that the "route" being added is indeed our willingly typed (for demonstration purposes) wrong URL:
2014-07-03 11_35_05-newAction.png

2014-07-03 11_35_05-newAction.png (9.29 KiB) Viewed 5329 times


What happens next, the program flow goes to factory.php where there's this code:
2014-07-03 11_41_30-factory.png

2014-07-03 11_41_30-factory.png (23.26 KiB) Viewed 5329 times

As it's easily seen, it's building a "convention over configuration" URL consisting of a wannabe controller... but we got our 99-bears in it!

Therefore the next:

Code: Select all

if (file_exists( $file ) && is_file( $file ))
fails and then the control goes to:

Code: Select all

$i = strrpos( $route, '/' );
		if ($i===FALSE) {
			trigger_error("Cannot find controller class file for route '$route'");
			exit;
		}
where $i of course is FALSE and thus shows the awful blank page with error.


Now, all of this is behaving "as expected", in the sense that there's not a real code bug.

Yet on a macro scale, an end user who for any reason gets a typoed URL (or just an old Google result for a decommissioned product) or anything, he gets the ugly blank page instead of the "internet specifications correct page for when and URL is not found", that is the "graceful" OC 404 page.

I also see a "disconnect" in how the error code is managing the flow, because I'd expect something along the lines of code "failing in a graceful way" so that it returns to index.php and executes the "error/not_found" controller.

I hope to have been clear, if you still need credentials and stuff... I think the ones I gave you some time ago are still active for you.

Best regards and thanks for your patience!

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Thu Jul 03, 2014 8:30 pm

I think the issue might be with your '.htaccess' file, which is either missing, or has wrong contents.

It uses the OpenCart 404 page just fine, see e.g. my own website: http://www.mhccorp.com/xyz

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Thu Jul 03, 2014 11:16 pm

Well, I have Nginx, installed along the lines of this OpenCart-Nginx github project. It works magnificently for everything, I don't know what would be wrong in there to cause such behavior.

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Thu Jul 03, 2014 11:45 pm

You should ask the author of this github-forked OpenCart to install the equivalent of Apache's '.htaccess', especially this:

Code: Select all

# SEO URL Settings
RewriteEngine On
# If your opencart installation does not run on the main web folder make sure you folder it does run in ie. / becomes /shop/ 

RewriteBase /
RewriteRule ^sitemap.xml$ index.php?route=feed/google_sitemap [L]
RewriteRule ^googlebase.xml$ index.php?route=feed/google_base [L]
RewriteRule ^download/(.*) /index.php?route=error/not_found [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_URI} !.*\.(ico|gif|jpg|jpeg|png|js|css)
RewriteRule ^([^?]*) index.php?_route_=$1 [L,QSA]

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Fri Jul 04, 2014 4:02 pm

JNeuhoff wrote:You should ask the author of this github-forked OpenCart to install the equivalent of Apache's '.htaccess', especially this:
I have checked, it's equivalent down to the last option ("L").

Basically what the above does, is some mapping between standard files (sitemap etc) and the OC controller delivering them. Then it checks that the requested path exists as directory or as file or as as specific extension and if it doesn't then it puts the URL path behind index.php?_route_= and then appends the query string (if any).

Basically, for what I can imagine, given a

http://www.domain.tld/99-bears

URL, Apache would just translate it into:

http://www.domain.tld/index.php?_route_=99-bears

which is exactly what I get with Nginx.

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Fri Jul 04, 2014 5:08 pm

So do you have the same ;problem on an Apache server, too?

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Fri Jul 04, 2014 5:29 pm

I am posting all of the above, not (just) because imo a pretty end user page trumps ugly blank pages all day long, but because I am installing Yii Embed for Opencart and of course (!) it just HAD to use the "not found" mechanism to work.

You might wonder who could be crazy enough to install 60 vQMods + OE + Yii on a website, but hey, I have to implement a whole (OC integrated) super-custom CRM, messenger and so much more that OC "simple" MVC costs me too much in terms of reinventing the wheel with the plethora of code I'd have to write to supply to the lack of "evolved" visual controls, validators and so on.

Anyway this is the kind of code I have found in an original OC install and which I'd have hoped to see managing my requests: the front.php file contains this:

Code: Select all

	private function execute($action) {
		if (file_exists($action->getFile())) {
			require_once($action->getFile());
			
			$class = $action->getClass();

			$controller = new $class($this->registry);
			
			if (is_callable(array($controller, $action->getMethod()))) {
				$action = call_user_func_array(array($controller, $action->getMethod()), $action->getArgs());
			} else {
				$action = $this->error;
			
				$this->error = '';
			}
		} else {
			$action = $this->error;
			
			$this->error = '';
		}
		
		return $action;
The above is a testament to Daniel's genius at doing so much with so little and (apparently simple) code.



The difference I have noticed once installed OE, is factory.php implementing some features like:

Code: Select all

public function newAction( $route, $args=array() ) {
         $properties = $this->actionProperties( $route, $args );
         return new Action( $properties );
     }
In particular, $this->actionProperties() is exactly the code (not present in original OC) that throws the error.


Here's what Yii itself dumps about attempting to load its index page, whose controller is "virtual" (that is the file DOES NOT exist) and that in original OC is loaded by index.php's:

$controller->dispatch($action, $factory->newAction('error/not_found'));

Yes, I know this is not exactly an "orthodox" way of instancing controllers but hey, I did not create the Yii OpenCart extension so I have little choice. Yii in fact is "left off" (to not unnecessarily load the OC loading times the majority of time it's not needed) and only instanced when it's controller, site/index is invoked. A vQMod (by its same author) changes error/not found handler to intercept calls to such controller before passing control back to error/not found.

Important: I have the "controller not found" error since months, I have unzipped Yii's embedded install just 1 hour ago, so no, it's not it.

In the next post I'll write why in my humble opinion there's an issue with actionProperties( $route, $args ).

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by dfumagalli » Fri Jul 04, 2014 5:42 pm

Basically actionProperties performs tight checks and in case they fail, the whole "stack" does not unwind to show a 404 page (which should ALWAYS happen, even if I pass a non existing controller) or, better, it does not unwind to invoking the error/not_found controller indicated in index.php.

Code: Select all

private function actionProperties( $route, $args ) {
    		$route = str_replace('../', '', (string)$route);

		$file = DIR_APPLICATION . 'controller/' . $route . '.php';
		if (file_exists( $file ) && is_file( $file )) {
			$class = $this->pathToClassName( 'Controller', $route );
			return array( 'file'=>$file, 'class'=>$class, 'method'=>'index', 'args'=>$args );
		} 

		$i = strrpos( $route, '/' );
		if ($i===FALSE) {
			trigger_error("Cannot find controller class file for route '$route'");
			exit;
		}
		$method = substr( $route, $i+1 );
		$filepath = substr( $route, 0, $i );
		if ($filepath===FALSE) {
			trigger_error("Cannot find controller class file for route '$route'");
			exit;
		}

		$file = DIR_APPLICATION . 'controller/' . $filepath . '.php';
		if (file_exists( $file ) && is_file( $file )) {
			$class = $this->pathToClassName( 'Controller', $filepath );
			return array( 'file'=>$file, 'class'=>$class, 'method'=>$method, 'args'=>$args );
		}

		trigger_error("Cannot find controller class file for route '$route'");
		exit;
	}
Instead it exits the program completely, with the white page error message. In NO situation should the program exit with such page. I mean, even if you did not agree with my conclusions AT ALL, this function, regardless whether using Apache or Nginx:

- is private so I can't "cleanly" inherit and change it via PHP OOP.

- It ALWAYS exits with blank page instead of redirecting to a "nice" error page and this is not good for end users.

- Exiting like that, prevents the "catch all" error/not found handler from being executed and this makes my Yii thing not work and prevents the nice-to-see 404 error page from being shown.

Edit: added exact stack trace.
2014-07-04-10_50_45-User-notice.png

2014-07-04-10_50_45-User-notice.png (68.44 KiB) Viewed 5291 times


Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Fri Jul 04, 2014 6:46 pm

I still think there is something wrong with your '.htaccess' equivalent in your NGINX server. It should never call the $factory->newAction method for the '99-bears' path. Take a look at this code in the index.php:

Code: Select all

// Front Controller 
$controller = new Front($registry);

// Maintenance Mode
$controller->addPreAction($factory->newAction('common/maintenance'));

// SEO URL's
$controller->addPreAction($factory->newAction('common/seo_url'));

// Router
if (isset($request->get['route'])) {
	$action = $factory->newAction($request->get['route']);
} else {
	$action = $factory->newAction('common/home');
}
Since there is no 'route' in the query string (only a '_route_=99-bears'), it creates the following actions:

'common/maintenance', 'common/seo_url', 'common/home'. All of them have valid controllers. As soon as it executes the 'common/seo_url' pre-action, it adds a 'route=error/not_found' which again has a valid controller, leading to OpenCart's error/not_found page, as expected.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Fri Jul 04, 2014 6:46 pm

Another, very tangible reason why imo there's something "architecturally" wrong as well:

- OC works with a chain of (pre)actions.

- A dispatcher proceeds looping through them. In case of wrong URL it invokes the special action "error/not_found".

- Program ends.


As of now, with OE installed this is what happens:

- OC works with a chain of (pre)actions.

- Program CAN end with blank error page before the flow even gets to the dispatcher (see the function I discussed in the previous post). Program may end here.

This is the "plastic representation" of this happening:
2014-07-04 11_34_23-Dispatcher.png

2014-07-04 11_34_23-Dispatcher.png (36.3 KiB) Viewed 5285 times

- Dispatcher may or may not be reached at all. If it does, it performs as "vanilla OpenCart" only if it can be reached.


Imo the original program flow is very linear and ends always putting the full control in the dispatcher. The OE modified "can abort early" program flow can abort before starting the dispatcher and this prevents a full control of the situation (including errors).

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by dfumagalli » Fri Jul 04, 2014 6:52 pm

JNeuhoff wrote:I still think there is something wrong with your '.htaccess' equivalent in your NGINX server. It should never call the $factory->newAction method for the '99-bears' path. Take a look at this code in the index.php:

Since there is no 'route' in the query string (only a '_route_=99-bears')
Here's my actual rewrite:

Code: Select all

location @handler {
                # more_set_headers 'Location 20: store @handler';
                rewrite ^/(.+)$ /index.php?_route_=$1 last;
        }
The "last" is the "L" option in the .htaccess, the _route_ is really there. The "more_set..." is just commented debug code.

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Fri Jul 04, 2014 7:00 pm

As I said, I am not familiar with NGINX, but for an url such as www.example.com/99-bears you should NOT get a route=99-bears in OpenCart's index.php, $request->get['route'] does not exist, hence it should never call the line

$action = $factory->newAction($request->get['route']);

because the code is like this:

Code: Select all

// Router
if (isset($request->get['route'])) {
	$action = $factory->newAction($request->get['route']);
} else {
	$action = $factory->newAction('common/home');
}

User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Fri Jul 04, 2014 7:40 pm

I have started modifying factory.php (if I recall correctly it's vQMod protected so I can't create a mod for it).

Adding some code to the factory.php actionProperties() function allows me to remove the ugly blank error screen in any situation, be it Nginx or whatever. The little addition past the comment is all what's needed to cover the "site/index" alike kind of errors, later I'll do something similar to cover the "99-bears" other if() branch.

I am posting this because we are many who use Nginx so why not help everybody?

Code: Select all

		$file = DIR_APPLICATION . 'controller/' . $filepath . '.php';
		if (file_exists( $file ) && is_file( $file )) {
			$class = $this->pathToClassName( 'Controller', $filepath );
			return array( 'file'=>$file, 'class'=>$class, 'method'=>$method, 'args'=>$args );
		}

		// 2014-07-04 | dfumagalli | Return OpenCart 404 page if file not found
		$filepath = 'error/not_found'; 
		$file = DIR_APPLICATION . 'controller/' . $filepath . '.php';
		$class = $this->pathToClassName( 'Controller', $filepath );
		return array( 'file'=>$file, 'class'=>$class, 'method'=>$method, 'args'=>$args );
Of course I could refactor it to not repeat the various statements etc. etc. but I just wanted a "proof of concept" that the ugly page can go away with no side effects.

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Fri Jul 04, 2014 7:52 pm

You could use this modified method in the factory.php:

Code: Select all

	private function actionProperties( $route, $args ) {
		$route = str_replace('../', '', (string)$route);

		$file = DIR_APPLICATION . 'controller/' . $route . '.php';
		if (file_exists( $file ) && is_file( $file )) {
			$class = $this->pathToClassName( 'Controller', $route );
			return array( 'file'=>$file, 'class'=>$class, 'method'=>'index', 'args'=>$args );
		} 

		$i = strrpos( $route, '/' );
		if ($i===FALSE) {
			$this->log->write("Cannot find controller class file for route '$route'");
			return array( 'file'=>'', 'class'=>'', 'method'=>'index', 'args'=>$args );
		}
		$method = substr( $route, $i+1 );
		$filepath = substr( $route, 0, $i );
		if ($filepath===FALSE) {
			$this->log->write("Cannot find controller class file for route '$route'");
			return array( 'file'=>'', 'class'=>'', 'method'=>'index', 'args'=>$args );
		}

		$file = DIR_APPLICATION . 'controller/' . $filepath . '.php';
		if (file_exists( $file ) && is_file( $file )) {
			$class = $this->pathToClassName( 'Controller', $filepath );
			return array( 'file'=>$file, 'class'=>$class, 'method'=>$method, 'args'=>$args );
		}

		$this->log->write("Cannot find controller class file for route '$route'");
		return array( 'file'=>'', 'class'=>'', 'method'=>'index', 'args'=>$args );
	}
That makes it more like OpenCart's original logic, and should work for your NGINX server.

Having said that, you should still fix your NGINX issue, it should NOT generate a 'route=99-bear' from your http://www.example.com/99-bear url! Instead, it should generate a '_route_=99-bear'!

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Sat Jul 05, 2014 12:28 am

Thank you for your code! I am going to check it out tomorrow or on Monday.

In the mean time, I am posting the $request I see in my debugger if I enter:

http://www.domain.tld/site/index


Isn't it showing _route_ as you ask?
2014-07-04 17_23_10-Route.png

2014-07-04 17_23_10-Route.png (44.74 KiB) Viewed 5262 times


Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Sat Jul 05, 2014 1:44 am

If you really have a _route_=site/index generated by NGINX, then it shouldn't do the

$action = $factory->newAction($request->get['route']);

from the index.php and you should not have seen the error message in a separate screen, rather, it should have used OpenCart's error/not_found page!

Therefore, something else might be wrong with your setup, which has nothing to do with OpenCart, nor with the Override Engine.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by dfumagalli » Sat Jul 05, 2014 6:36 pm

dfumagalli wrote:Thank you for your code! I am going to check it out tomorrow or on Monday.
JNeuhoff wrote:You could use this modified method in the factory.php:

That makes it more like OpenCart's original logic, and should work for your NGINX server.

Having said that, you should still fix your NGINX issue, it should NOT generate a 'route=99-bear' from your http://www.example.com/99-bear url! Instead, it should generate a '_route_=99-bear'!
Thank you so much, the code you posted fixed most of my issues in one shot!

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by dfumagalli » Sat Jul 05, 2014 7:30 pm

... aaaaaaand since I did not want to leave any open question marks... I haveeeeee finally found the REAL culprit!

While banging my head into the wall, I went to clear the logs. In fact your new code writes in the log, which by the way is very nice because it lets me track the 404 error offending URLs.

I went to clear the vQMod logs as well... when suddenly I realized it was aborting... and guess what? Exactly while processing the newest addition, that is exactly the Yii Embed engine.
The Yii Embedded engine, like most mods, comes with an accompanying vQMod and for some reason it was aborting. So, while the "bad URL" issue was solved by your code, nothing about the embedded Yii engine worked.

Among the various bits that vQMod, it adds one line to system/response.php right before the final: "echo $output;".

INCREDIBLE to see, if was failing to find such puny string: echo $output;

This is what the vQMod would show: 0 matches for echo $output;
2014-07-05 12_09_15-Response.png

2014-07-05 12_09_15-Response.png (48.46 KiB) Viewed 5220 times

I went mad searching for possible OE mods (I have REALLY tweaked everything possible in this website) which could inherit and change that, I went to seek for other conflicting vQMods... and in the end, the simple truth is...

... in the official 1.5.6 (August 15) distribution... whoever wrote the code typoed $output...

Code: Select all

	public function output() {
		if ($this->output) {
			if ($this->level) {
				$ouput = $this->compress($this->output, $this->level);
			} else {
				$ouput = $this->output;
			}	
				
			if (!headers_sent()) {
				foreach ($this->headers as $header) {
					header($header, true);
				}
			}
			
			echo $ouput;
		}
	}
So, who entered the code, created a local $ouput variable missing a "t" and this in turn failcascated the whole castle of cards sitting above. :o

I guess in previous and subsequent OpenCart versions that typo got fixed, so the vQMod relied on the correct spelling which my distribution did not have...

Heavy OpenCart Customizations. Current project in progress: fleurworld.com


Active Member

Posts

Joined
Tue Aug 20, 2013 3:34 am

Post by JNeuhoff » Sat Jul 05, 2014 8:07 pm

It got fixed from 1.5.6.x onwards. This VQmod issue illustrates why the Override Engine is a better approach to modifying core OpenCart classes :)

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am

Who is online

Users browsing this forum: No registered users and 3 guests