Now that Drupal can install an SQLite database, using the "database name" field as a filepath, there is no limit on the length of a database name.

Which means this #maxlength property in install.php should probably be removed:

$form['basic_options']['database'] = array(
'#type' => 'textfield',
'#title' => st('Database name'),
'#default_value' => empty($database['database']) ? '' : $database['database'],
'#size' => 45,
'#maxlength' => 45,
'#required' => TRUE,
'#description' => st('The name of the database your @drupal data will be stored in. It must exist on your server before @drupal can be installed.', array('@drupal' => drupal_install_profile_name())),
);

If you place the sqlite file into the sites directory, it's pretty easy to run into that length limit. Since it's inconsistent between database types (MySQL has 32, not 45, by the way), just remove the client-site max_length entirely - you can validate the name server-side after the type has been selected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka’s picture

I meant to put that in PHP, not quote tags, of course.


    $form['basic_options']['database'] = array(
      '#type' => 'textfield',
      '#title' => st('Database name'),
      '#default_value' => empty($database['database']) ? '' : $database['database'],
      '#size' => 45,
      '#maxlength' => 45,
      '#required' => TRUE,
      '#description' => st('The name of the database your @drupal data will be stored in. It must exist on your server before @drupal can be installed.', array('@drupal' => drupal_install_profile_name())),
    );

cburschka’s picture

Status: Active » Needs review
FileSize
2.56 KB

The user, password, host and port are likewise limited to 45. All of these limits are arbitrary, since they don't match the specifications (hostnames can be 255 characters, users 16 characters, ports up to 65536 and therefore 5 digits). Being arbitrary, the limits therefore are completely useless for input validation. As the variables are stored in a settings file, there are no database field lengths to watch out for either. I think this makes the limits redundant; please correct me if I missed something.

Therefore, this patch removes the limits defined by the database (which can vary based on the type) and replaces the limits defined by common standards, ie. the hostname to 255 and the port to 5.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Makes complete sense, and the patch looks great. Thanks Arancaytar.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Dave Reid’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

pwolanin’s picture

Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Patch (to be ported)

why not backport this fix to 6? Sounds like it's a bug not a feature.

cburschka’s picture

Priority: Normal » Minor
Status: Patch (to be ported) » Needs review
FileSize
2.19 KB

The limits are also arbitrary in D6, but without the file paths of SQLite there is no way to exceed the arbitrary limits (the natural limits are smaller). So it's only a cosmetic change.

Nevertheless, here is a patch for D6.

alexanderpas’s picture

Status: Needs review » Reviewed & tested by the community

works.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed to Drupal 6, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.