Prevent url() from expanding aliases while the installer is running. [#1149580]

Problem/Motivation

When called with default options, the url() function expands aliases and will fail if invoked before Drupal has been fully installed. When modules are being installed from an install profile, their installation code may be invoked before all of the required database tables have been created. Therefore, the url() function should not be used within any code that may be invoked from an install profile, unless its alias parameter is set to TRUE.

Possible resolutions

#22: This limitation of the url() function should be documented in its function header.
#25: The documentation for the alias option should warn that it must be set to TRUE for code that may be invoked at install time.
#27: The alias option should automatically default to TRUE at install time.
#21: Every Drupal core function should document the possible scope(s) in which it may safely be used.
#23: The install profile documentation should indicate which core functions may or may not be safely invoked at install time.
#20: Relevant hook_foo() api documentation should indicate which core functions may or may not be safely invoked therefrom.
#32: Prevent alias expansion while the installer is running, regardless of the $options['alias'] value.
#1311774: Use a path.inc replacement while the installer is running.

Remaining tasks

A consensus must be reached and acted upon.

User interface changes

None.

API changes

None.

Original report

I recently discovered that using the url() function in a hook_requirements() implementation will trigger the following cryptic error and abort the installation:

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'd7.url_alias' doesn't exist

If this behavior is correct and by design, it should be mentioned somewhere in the documentation.

Comment	File	Size	Author
#32	url-breaks-in-profiles-1149580-32.patch	2.15 KB	pillarsdotnet
#32
#27	url-breaks-in-profiles-1149580-27.patch	1.49 KB	pillarsdotnet
#27
#25	url-breaks-in-profiles-1149580-25.patch	1.19 KB	pillarsdotnet
#25
#22	url-breaks-in-profiles-1149580-22.patch	1.01 KB	pillarsdotnet
#22
#19	url-breaks-in-profiles-1149580-19.patch	1.23 KB	pillarsdotnet
#19
#18	url-breaks-in-profiles-1149580-18.patch	1.22 KB	pillarsdotnet
#18
#17	url-breaks-in-profiles-1149580-17.patch	1.22 KB	pillarsdotnet
#17
#15	url-breaks-in-profiles-1149580-15.patch	1.19 KB	pillarsdotnet
#15
#10	url-breaks-in-profiles-1149580-10.patch	1.11 KB	pillarsdotnet
#10
#13	url-breaks-in-profiles-1149580-13.patch	1.01 KB	pillarsdotnet
#13

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

pillarsdotnet CreditAttribution: pillarsdotnet commented 6 May 2011 at 19:56

Note that the above error only occurs if the module in question is installed through a custom profile. When installing manually, through the admin/modules page, there is no error.

Comment #2

pillarsdotnet CreditAttribution: pillarsdotnet commented 6 May 2011 at 20:07

I tried inserting the following code just before the url() call:

module_enable(array('system'), TRUE);

The following error message aborted the installation:

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'd7.system' doesn't exist

Comment #3

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 9 May 2011 at 15:29

Status:

Active

» Postponed (maintainer needs more info)

So you are saying we should document that url() cannot be used in hook_requirements()? Or is this a core code bug?

Comment #4

pillarsdotnet CreditAttribution: pillarsdotnet commented 9 May 2011 at 16:18

Status:

Postponed (maintainer needs more info)

» Active

You can use url() in hook_requirements(), as long as your module will never be installed through an installation profile.

I don't know if the behavior is a bug or by design.

If it is a bug, then it is a core bug. If it is intentional, then it may already be documented *somewhere*.

Therefore I submitted it (first) as a documentation bug, in the hopes that if this *is* documented somewhere, then someone on the documentation team would know about it.

I expect one of the following outcomes:

This behavior is already documented, and this issue should be closed (works as designed) with a pointer to the relevant documentation.
This behavior is intentional but undocumented, and this issue should be needs work pending a documentation patch which I would be happy to provide.
This behavior is a bug, and the issue component should be changed from documentation to install system, and the version bumped to 8.x.

Should I have submitted this as a support request? It is my experience that support requests against Drupal core get ignored.

Comment #5

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 9 May 2011 at 16:48

Version:	7.x-dev	» 8.x-dev
Component:	documentation	» base system

OK, let's move this to base system, and see what they say (whether it's by design/doc issue or a core issue).

In either case, it needs to be addressed in Drupal 8 first at this point, then backported to 7.

Comment #6

pillarsdotnet CreditAttribution: pillarsdotnet commented 9 May 2011 at 17:09

Issue tags:

+Needs backport to D7

Comment #7

pillarsdotnet CreditAttribution: pillarsdotnet commented 20 May 2011 at 21:01

Posted links to this issue on both the url() and hook_requirements() API pages.

Comment #8

girishmuraly CreditAttribution: girishmuraly commented 30 June 2011 at 15:50

Ran into a similar issue before. Basically when enabling a module during installation phase (profile), most of the drupal functions are not available yet. Hence unfortunately I needed to use workarounds and more basic functions. Use the $phase argument to check for the phase you are in.

Here is an example from the superfish module:

function superfish_requirements($phase) {
 if ($phase == 'install' && !drupal_get_path('module', 'libraries')) {
    $requirements['superfish']['severity'] = REQUIREMENT_ERROR;
    $requirements['superfish']['description'] = $t('Superfish module requires the <a href="@url">Libraries module</a> to be installed.', array('@url' => 'http://drupal.org/project/libraries'));
  }

Comment #9

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno commented 15 October 2011 at 02:36

Does the error happen also when $phase is equal to "runtime"?

Comment #10

pillarsdotnet CreditAttribution: pillarsdotnet commented 15 October 2011 at 18:37

File	Size
url-breaks-in-profiles-1149580-10.patch	1.11 KB

@#9 by kiamlaluno on October 15, 2011 at 2:36am:

Does the error happen also when $phase is equal to "runtime"?

Of course not. It only happens when invoked from an install profile, in which case $phase would be equal to "install". The confusing thing is that when installing modules via the administration screens (not from a profile), the error does not occur, even though $phase would also be equal to "install" in such a case.

I'm going to call this a documentation problem. Here's a patch.

Comment #11

pillarsdotnet CreditAttribution: pillarsdotnet commented 15 October 2011 at 13:27

Title:	The url function cannot be used in hook_requirements() for modules installed with a profile.	» Document that the url() function should not be called from an install profile.
Component:	base system	» documentation
Status:	Active	» Needs review

Summary updated.

Comment #12

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 15 October 2011 at 17:35

Documentation seems to be a reasonable course of action, and thanks for the patch!

After reading through this issue thread in some detail, though, I'm wondering if "full bootstrap" is actually the problem though. It seems like the problem was that the tables didn't exist yet, not the bootstrap phase? Can you clarify my understanding at all? I just don't want to use the phrase "requires a full bootstrap" unless that is really the problem. Maybe it should just say "This function should not be called from functions that may be...".

And I am also wondering if this information should be in a separate paragraph rather than being tacked onto the suggestion about using l() instead? It doesn't seem related to me.

Thoughts?

Comment #13

pillarsdotnet CreditAttribution: pillarsdotnet commented 15 October 2011 at 18:05

File	Size
url-breaks-in-profiles-1149580-13.patch	1.01 KB

More accurate description moved to separate paragraph.

Comment #14

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 15 October 2011 at 21:48

Status:

Needs review

» Needs work

Since this new information is much less often needed (I think) than the l() suggestion, can we leave the l() information that was there first?

Also, table names should be in {} in documentation, because people can use table prefixes.

The other thing is... When I read this new paragraph, assuming I hadn't read this issue first, my reaction was that this was a giant logical leap: Huh? How is url_alias table connected to install profiles?!? I don't think we want to explain the whole chain of logic, but... And maybe it would be good to start with the "Don't use this in xyz" and then continue with an explanation that is not missing huge steps in logic? I don't know how to reword it, but the explanation there didn't really satisfy me.

Comment #15

pillarsdotnet CreditAttribution: pillarsdotnet commented 15 October 2011 at 22:46

Status:

Needs work

» Needs review

File	Size
url-breaks-in-profiles-1149580-15.patch	1.19 KB

Is this better?

Comment #16

sven.lauer CreditAttribution: sven.lauer commented 16 October 2011 at 03:37

This issue is even more far-reaching: hook_install() implementations are free to invoke functions that have nothing to do with installation (the whole .module file is available to them). So maybe we should say:

Do not use this function within functions that may be invoked from an install profile, such as implementations of hook_enable(), hook_install(), or hook_requirements('install') or functions called by those.

Comment #17

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 04:04

File	Size
url-breaks-in-profiles-1149580-17.patch	1.22 KB

Is this better?

Comment #18

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 04:06

File	Size
url-breaks-in-profiles-1149580-18.patch	1.22 KB

Minor correction in grammar.

Comment #19

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 06:56

File	Size
url-breaks-in-profiles-1149580-19.patch	1.23 KB

And another.

Comment #20

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 16 October 2011 at 20:06

I'm wondering about the wisdom of this change. There are a whole bunch of functions that probably shouldn't be called from these places, not just URL. Wouldn't it be better to document this on hook_enable(), hook_install(), and hook_requirements('install') -- and say something like "Don't call functions that access the database" there?

Comment #21

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 20:47

@#20 by jhodgdon on October 16, 2011 at 8:06pm:

Wouldn't it be better to document this on hook_enable(), hook_install(), and hook_requirements('install') -- and say something like "Don't call functions that access the database" there?

Yes, this limitation should definitely be documented there, also.

Absent such a warning, a naïve user such as myself may incorrectly assume that url() and l() would have no need to access the database in order to format an HTML hyperlink, or that a profile would install core /modules before sites/all/modules.

In fact, I would like to propose that in some future version of Drupal, all core functions must indicate the scope(s) for which their use is valid:

Global (no dependencies).
Partial bootstrap (specify the minimum level).
Installer profiles.
(etc.)

Comment #22

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 20:53

File	Size
url-breaks-in-profiles-1149580-22.patch	1.01 KB

Warning reduced to a bare minimum.

Comment #23

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno commented 16 October 2011 at 20:53

Actually, url() doesn't always access the path alias table. In fact, it contains the following code:

elseif (!empty($path) && !$options['alias']) {
    $language = isset($options['language']) && isset($options['language']->language) ? $options['language']->language : '';
    $alias = drupal_get_path_alias($original_path, $language);
    if ($alias != $original_path) {
      $path = $alias;
    }
}

If I set $options['alias'] to TRUE, drupal_get_path_alias() will not be called.

I think it makes more sense to add a warning about invoking functions that could cause a database access in installation profiles.

Comment #24

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 21:15

I think it makes more sense to add a warning about invoking functions that could cause a database access in installation profiles.

Unfortunately, the installation profile author has no control over the contents of $module.install files. I doubt, therefore, that adding a warning to the installation profile documentation would be effective.

Although specific people (such as yourself) may have more or less understanding of certain functions, I can't find a single reference anywhere that lists which functions may be called from which circumstances.

Certainly the Module developer's guide to writing .install files (both 6.x and 7.x versions) is no help.

Comment #25

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 21:23

File	Size
url-breaks-in-profiles-1149580-25.patch	1.19 KB

Moved the warning to the alias option.

Comment #26

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 21:25

Title:

Document that the url() function should not be called from an install profile.

» Document that the 'alias' option must be TRUE when the url() function may be invoked during install time.

Comment #27

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 21:31

Title:	Document that the 'alias' option must be TRUE when the url() function may be invoked during install time.	» Set the url() default 'alias' option to drupal_installation_attempted() to avoid confusing fatal database errors.
Component:	documentation	» base system

File	Size
url-breaks-in-profiles-1149580-27.patch	1.49 KB

Better yet, simply set the default for 'alias' to drupal_installation_attempted().

Comment #27.0

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 21:31

Issue summary:

View changes

Updated from issue summary template.

Comment #28

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 22:45

Yay, #27 passes tests!

Note that if this approach is selected, a corresponding test should be written to ensure that it stays fixed.

Comment #29

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 16 October 2011 at 22:58

Issue tags:

-Needs backport to D7

Removing backport tag. This is not d7 material any more. I'm also unsubscribing. :)

Comment #29.0

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 16 October 2011 at 22:58

Issue summary:

View changes

Various possible solutions listed.

Comment #29.1

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 23:09

Issue summary:

View changes

Added limiting clause to introductory sentence.

Comment #30

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 23:13

Issue tags:

+Needs backport to D7

I don't see why at least one of the possible resolutions couldn't be backported to D7.

Comment #30.0

pillarsdotnet CreditAttribution: pillarsdotnet commented 16 October 2011 at 23:13

Issue summary:

View changes

And another limiting clause at the end of the first paragraph.

Comment #31

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 17 October 2011 at 02:24

There are a whole bunch of odd bugs that can show via installation profiles that never show when enabling modules normally, so I'd be in favor of expanding the documentation and this patch.

Comment #32

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 03:02

File	Size
url-breaks-in-profiles-1149580-32.patch	2.15 KB

Another option suggested by chx in IRC: skip alias expansion during install mode, regardless of the $options['alias'] setting.

Comment #32.0

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 03:02

Issue summary:

View changes

typo

Comment #33

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 03:04

Title:

Set the url() default 'alias' option to drupal_installation_attempted() to avoid confusing fatal database errors.

» Prevent url() from expanding aliases while the installer is running.

Comment #33.0

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 03:04

Issue summary:

View changes

Another possible resolution.

Comment #34

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 12:31

Status:

Needs review

» Closed (duplicate)

Closing in favor of #1311774: Prevent path functions from using database tables that have not yet been created.

Comment #34.0

pillarsdotnet CreditAttribution: pillarsdotnet commented 17 October 2011 at 12:31

Issue summary:

View changes

Another possible resolution.