The includes are wrong and don't work in php5 (for me at least), generating a host of open_basedir errors.

These lines:

include_once(drupal_get_path('module', 'pathauto') .'/pathauto_menu.inc');
include_once(drupal_get_path('module', 'pathauto') .'/pathauto_node.inc');
include_once(drupal_get_path('module', 'pathauto') .'/pathauto_taxonomy.inc');
include_once(drupal_get_path('module', 'pathauto') .'/pathauto_user.inc');
include_once(drupal_get_path('module', 'pathauto') .'/contrib/pathauto_node_event.inc');
include_once(drupal_get_path('module', 'pathauto') .'/contrib/pathauto_node_i18n.inc');

should be replaced with:

include_once('pathauto_menu.inc');
include_once('pathauto_node.inc');
include_once('pathauto_taxonomy.inc');
include_once('pathauto_user.inc');
include_once('contrib/pathauto_node_event.inc');
include_once('contrib/pathauto_node_i18n.inc');

This is in line with the determined 'correct' way to include in Drupal modules.

CommentFileSizeAuthor
#20 pathauto_includes_path.patch827 bytesgreggles
#7 include_0.patch1.3 KBfgm
#6 include.patch1.96 KBfgm
#4 pathauto.module_12.patch1.13 KBniklp

Comments

niklp’s picture

niklp commented

OOPS!

Sorry, the blocks of code are the wrong way round! The top block illustrates the CORRECT method.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

NikLP - can you clarify where you get those errors? In the apache logs? In your watchdog tables? In...?

And please see http://drupal.org/diffandpatch so you can create a proper diff file.

Thanks.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Priority: Critical » Normal

And it's not critical.

niklp’s picture

niklp commented
Status: Active » Needs review
StatusFileSize
new pathauto.module_12.patch1.13 KB

The errors occur on every page until the fix is made, as the problem arises when the system attempts to include the pathauto module. They are also listed in watchdog, for what it's worth, but to see that, you'd have to scroll past the screenful of errors that were there already! :)

Patch attached. Works dandy for me. I'm assuming this is purely because of some setting based around open_basedir. This occurs under php5 only AFAIK.

Sorry for getting the status wrong - I assumed that a boatload of errors would kinda be classed as critical.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

It would be critical if it affected a lot of people, but this is the first report of the problem.

Thanks for the patch, I'll test it out today. Hopefully some other folks can test it as well, but it looks reasonable enough that I will commit it in a day or two without any other testers.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
StatusFileSize
new include.patch1.96 KB

Maybe it would be more efficient to cache the path like in this version ?

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
StatusFileSize
new include_0.patch1.3 KB

Sorry, wrong patch format.

niklp’s picture

niklp commented

That's now blindingly obvious. My mistake.

However, I note with a hint of schadenfreude that you have used double quotes, which ironically use more processor time than single quotes. You would be better to write (an individual line):

include_once($pathauto_path . '/pathauto_menu.inc');

...as you'd be saving a tiny bit of time on each of the includes...! :)

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I believe that the double quotes vs. single quotes problem was fixed a few minor versions of php ago so that they are now the same speed.

@fgm - did you also test the patch? any advice on the status as working or not for you?

Greg

fgm’s picture

fgm
Primary language French
Location Paris, France
commented

Actually no: client-side I work on windows, and patch.exe just doesn't work, so I can only generate patches, not test them. I suggested it because it seemed wasteful to repeat the function call.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

This worked for me and seems like a small enough change.

Thanks to you both for the original report/patch and for the slight performance enhancement. It does weigh on me that those happen for every page load...

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Fixed » Patch (to be ported)

We should do this for 4.7 as well.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Version: 5.x-1.x-dev » 4.7.x-1.x-dev
Status: Patch (to be ported) » Postponed (maintainer needs more info)

Actually, we may not need to do this for 4.7 because of the way that the files are included there.

I'm leaving this as "needs more info" and assigning it to 4.7 so that we can know what to do if someone else comes across it when using 4.7.

The error would read something like

" * warning: main(modules/pathauto/pathauto_menu.inc)
[function.main]: failed to open stream: No such file or directory in
/home/trafficnoise/public_html/drupal/modules/pathauto/pathauto.module
on line 7."

marius.schebella’s picture

marius.schebella commented

I am on v 5.x.1.x-dev (from jan30) and drupal 5.0, and also had problems with warnings (no such file directory...) I changed the file pathauto.modules according to niklp's instructions and it works good for me now.
marius.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

marius - can you provide the complete error message that you get? It would be helpful for us.

niklp’s picture

niklp commented

Sounds like Marius is referring to the same initial error that I mentioned - the open_basedir errors include "directory not found"s, IIRC.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Version: 4.7.x-1.x-dev » 5.x-1.0

NikLP and marius - I guess I'm confused because the version of pathauto that you said you were using 5.x-1.x-dev from Jan 30 should have included the fix that NikLP originally proposed. So, I don't understand how Marius could have edited the file to implement your changes...

EXCEPT if Marius did the changes like you suggest in the very first post which were to more or less reverse the patch that went it.

So, the question becomes: did the patch I applied in #11 cause a regression or was Marius using a different version than originally thought?

Marius - if you could download the current 5.x-1.x-dev tarball and try it out without modifications that would be very helpful:
http://ftp.osuosl.org/pub/drupal/files/projects/pathauto-5.x-1.x-dev.tar.gz

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Postponed (maintainer needs more info) » Active

We apparently need to do some more work on getting this to work right...

See http://drupal.org/node/129841

niklp’s picture

niklp commented

*sigh*

Ok, well for the record, I've had this working under the following:

mythic-beasts.com - php 4.4.3 (suExec as CGI), mysql 4.1.7, apache 1 (vers. unsure)
my current host - php 5.0.2 (mod_php), mysql 4.1.14, apache 2.0.54

Both of these setups have used Drupal 5.1, and pathauto versions 1.0 and 1.1 concurrently with no problems.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs review
StatusFileSize
new pathauto_includes_path.patch827 bytes

Perhaps the solution in http://drupal.org/node/119475 will help:

Basically it involves using:

$module_dir = base_path() . drupal_get_path('module', 'amazon');

instead of

- $module_dir = drupal_get_path('module', 'amazon');

I haven't tested in myself (my servers seem to be immune to this problem anyway) but I'd appreciate it if folks who do have this problem could test it out.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

Most of the reports of problems came from a time when the releases were potentially mixed up. I think we can safely say that this is actually fixed.

If anyone is still having problems with this please try out the fix in #20 and reopen the issue.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)