Validate option and transient name lengths

[chrisbliss18]

The option_name column in the options table holds up to 64 characters yet there aren't any length checks to ensure that the length isn't exceeded. This leads to all sorts of odd behavior as the name will be truncated to fit, saving the option but not allowing it to be retrieved with the same name.

This issue affects all uses of the options system. A particular annoyance is transients. A transient that doesn't expire has a max name length of 53 characters yet a transient that does expire has a max name length of 45 characters. When attempting to save an expiring transient of name length between 46 and 53 characters, the transient will store but will be deleted before being used when get_transient is called due to the missing _transient_timeout_ option (since it was too long).

The core issues are that the functions don't return any type of failure condition on names that are too long and no warning is created. So, I've created a patch that addresses both of these issues.

The patch shows an example of how option and transient name length validation can be added. In order to allow users to modify the option_name column length, a new constant, WP_OPTION_LENGTH, is created with a default value of 64. This constant is checked for all length validation.

This is simply an example of how I'd like the validation to function. I'm not attached to the wording of the warnings or the name of the constant.

Related: #13310

We don't currently do sanity checks on the schema anywhere. If we are to do this, then it should be done across the board: #11959, #14649, etc. <​http://wpdevel.wordpress.com/2010/09/13/cpt-enhancements/#comment-10882>

I'm all for adding these checks. Even as a seasoned WP developer, running into these issues can cause massive problems and wasted time in order to remember that many functions make the final option name longer and that it is being truncated.

I know that there are many more areas to be tackled. I could have gone deeper with my original patch as I made a full listing of all the areas where option names are modified before being stored. However, I didn't want to jump off the deep end when opening this topic for discussion.

I see that both #11959 and #14649 are awaiting patches. I can working on patches for those.

First, I'd like to know:

1) Is this something that will be added if I work on it? I have a history of working on things that won't get added due to one reason or another, and I'd rather not waste my time.

2) What is the best way to manage the workflow? Should the existing tickets relating to sanity checks be handled on an individual basis or would making a large batch ticket with a massive patch be preferred. The former (individual tickets/patches) makes more sense to me, but I'm not on the core team.

@core developers: Any feedback? Im very interested to see a solution to this issue.

Attached patch restricts set_transient() only.

The rest of them aren't necessary -- update_option( $length_65 ) will cause the option name to be truncated, but MySQL (at least 5.5.8) will not truncate the WHERE clause in get_option( $length_65 ).

But for transients, the additional timeout prefix will cause a transient that's between 45 and 53 to lack any expiration. Luckily, because they can expire, then this patch won't do much harm.

In the mean time I've updated the Codex to make it clear that the transient key should be 45 characters or less:

​http://codex.wordpress.org/Function_Reference/set_transient#Parameters
​http://codex.wordpress.org/Transients_API#Saving_Transients

Why not md5 the transient key used if it's over X amount long?

Replying to sc0ttkclark:

Why not md5 the transient key used if it's over X amount long?

That could make it more difficult to debug things, but it's not a bad idea.

This ticket could use unit tests to proceed.

(Disregard that unit test, I added it by accident.)

Unit test that actually tests (probably) useful things added.

Just to correct one statement made by Chris - the transient name length is 45 characters, but only if you're not using network transients. If you use these, '_site' is prefixed to the name, meaning that the maximum length is now 40.

If we're going to limit and report on name length, would it make more sense to just make it a uniform 40 rather than have different limits for different transient types?

I vote uniformity, gives core 5 characters to use in the future for any future transient variations too.

A possible negative side effect of a uniform length limit is that it can create warnings for existing code that doesn't have any real issues. I'm not against the idea, but I wanted to make sure that this potential situation was noted.