node type named '0' breaks node module [#104352]

This is a sub-part of the larger patch proposed here: http://drupal.org/node/88633

Go ahead- I dare you. Create a new node type and give it the machine-readable name 0 (just the number zero).

This problem occurs because the validation for "required" form values allows the string '0', but in PHP empty('0') == TRUE, so all the checks for empty() (or not empty()) $type->type values don't work.

Attached patch returns a validation error if the $type->type evaluates to empty. This is the simplest thing I can think of that does not break any existing t() strings.

Comment	File	Size	Author
#6	empty_type_validate_3.diff	922 bytes	pwolanin
#6
#3	empty_type_validate_2.diff	969 bytes	pwolanin
#3
	empty_type_validate_1.diff	873 bytes	pwolanin

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Jaza CreditAttribution: Jaza commented 21 December 2006 at 04:35

Status:

Needs review

» Reviewed & tested by the community

This is about the simplest solution that we can implement for catching this uncommon edge case. I think that other possible solutions, such as enforcing a minimum length of 2 chars for node type names, is overdoing it.

RTBC.

Comment #2

drumm

he/him

NY, US

CreditAttribution: drumm commented 25 December 2006 at 09:44

Status:

Reviewed & tested by the community

» Needs work

The error message, "Invalid machine-readable name 0," is not helpful. Why isn't this valid? What can be done to fix it? Why is it 'machine-readable' in the message but the field is labeled 'type'?

Comment #3

pwolanin CreditAttribution: pwolanin commented 26 December 2006 at 01:47

Status:

Needs work

» Needs review

File	Size
empty_type_validate_2.diff	969 bytes

Here's an improved error message.

Comment #4

pwolanin CreditAttribution: pwolanin commented 29 December 2006 at 01:07

Status:

Needs review

» Reviewed & tested by the community

since the text seemed to be the only problem, setting back to RTBC

Comment #5

ChrisKennedy CreditAttribution: ChrisKennedy commented 29 December 2006 at 03:57

Status:

Reviewed & tested by the community

» Needs work

The comment should go on its own line imo.

More importantly, the error message is inaccurate: "Please use a type name with more than one character, or a character other than zero."

As far as I can tell there is no requirement that the type be more than one character. I think you're trying to say that it needs to be at least one character, and also cannot be '0'. However, I this message need only occur if the type is zero itself, because #required is already set to TRUE for $form['identity']['type'], so we know that it isn't a truly empty string. See _form_validate().

If I'm right, the error message should be something like: "Please choose a type name other than '0'." This also suggests that the comment should be modified, and you might as well make the if test be === '0' to be clearer.

Comment #6

pwolanin CreditAttribution: pwolanin commented 29 December 2006 at 04:46

Status:

Needs work

» Needs review

File	Size
empty_type_validate_3.diff	922 bytes

FYI, in form.inc, the code validating "required" fields in _form_validate() is:

      if ($elements['#required'] && empty($elements['#value']) && $elements['#value'] !== '0') {
        form_error($elements, t('!name field is required.', array('!name' => $elements['#title'])));
      }

revised patch attached for this annoying edge case.