Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
In the current code, the INSERT statement for uid 0 into {users} takes place inside of install.php.
As a result, the code in system.install that allows the auto_increment to accept a zero value:
db_query("SET sql_mode = 'NO_AUTO_VALUE_ON_ZERO'");
Does not get executed. As a result the anon user is assigned to uid 2.
Patch moves the insert statement into system.install where it can inherit the above SQL.
Comment | File | Size | Author |
---|---|---|---|
#28 | many_a_ways_to_skin_a_cat_5.patch | 6.34 KB | chx |
#22 | i172265.patch | 5.83 KB | David Strauss |
#20 | many_a_ways_to_skin_a_cat_4.patch | 5.82 KB | chx |
#15 | many_a_ways_to_skin_a_cat_3.patch | 5.8 KB | chx |
#11 | many_a_ways_to_skin_a_cat_2.patch | 1.82 KB | chx |
Comments
Comment #1
agentrickardShould be critical.
Comment #2
agentrickardTitle fix. Need sleep.
Comment #3
webchickI believe this is a duplicate of http://drupal.org/node/149540, which was just committed yesterday.
Comment #4
agentrickardNo. It doesn't seem to be. I downloaded HEAD from CVS last night and again this morning.
It may be that I just don't get the other thread. In my installation, uid 1 is created, and Anon is created as uid 2. the patch makes Anon be created as uid 0, as expected. If this is not required, then I don't understand and the rest of this post is meaningless and the issue can be closed.
This patch has no effect AFAIK on the auto_increment value.
The problem is that the SQL insert for anon is inside the install.php file, as a result, the no_zero fix discussed in the other issue does not work.
Using MySQL 4.1.13.
See function install_user_submit(), line 973:
The previous patch moved this insert statement out of system.install, but the no_zero code is only in system.install. Line 213:
For MySQL (4.1 at least), this statement _must_ precede the insertion of uid 0. It currently does not. It is easier and more efficient to move it to system.install than to move the IF MySQL check and sql_mode statement into install.php.
Comment #5
chx CreditAttribution: chx commentedHey this rolls back my solution to the uid 1 problem. If you do this then you might get uid1 as uid2 instead. Seems there is no better solution than db_query("SET sql_mode = 'NO_AUTO_VALUE_ON_ZERO'"); .
Comment #6
chx CreditAttribution: chx commentedI rolled back my patch as agentrickard here and enrolled David Strauss' http://drupal.org/node/149540#comment-255445 solution without triffling with user_save.
Comment #7
chx CreditAttribution: chx commentedhttp://drupal.org/node/149540#comment-255412 opsie. This won't work. I am studying this a bit more.
Comment #8
chx CreditAttribution: chx commentedComment #9
chx CreditAttribution: chx commentedNow with autologin of uid1 works.
Comment #10
chx CreditAttribution: chx commentedComments.
Comment #11
chx CreditAttribution: chx commentedusers.data is initialized into an empty array.
Comment #12
hunmonk CreditAttribution: hunmonk commentedcode style is good. tested on postgres, and it works w/ no errors. the approach has the advantage of being db agnostic, which is good. note this bit of weirdness that we'd have to put up with, though...
uid 2 is skipped b/c that auto-increment value was already used.
Comment #13
chx CreditAttribution: chx commentedThen we are good to go. Skipping uid 2 is something that HEAD already does, so we accepted that artifact earlier.
Comment #14
ChrisKennedy CreditAttribution: ChrisKennedy commentedPatch worked for me on MySQL 4.1 - ran into the problem because anonymous watchdog entries don't show up if the anonymous user gets uid=2.
Comment #15
chx CreditAttribution: chx commentedFollowing David Strauss' advice I removed the sql_mode as it's not needed now.
Comment #16
David StraussThe method in chx's patch is much safer than what's currently in the code because it eliminates sql_mode changes in MySQL. Using MySQL's less common features has proven unreliable, as we've seen from the experience with my non-locking db_next_id patch and the trouble with NO_AUTO_VALUE_ON_ZERO. As long as we're supporting everyday shared hosting environments, we must tread lightly.
What looks like a bunch of changes to the PostgreSQL code is really a minor refactoring. The MySQL-specific code is no longer necessary, and the patch's
if
statement is a more elegant way to handle the remaining PostgreSQL-specific code.Using zero for the anonymous uid still causes issues. The practice should be eliminated in Drupal 7, but that's too big of an architecture change for Drupal 6. This patch is the next best thing.
User 2 is an official collector's item now. :-)
Comment #17
chx CreditAttribution: chx commented"minor refactor" is changing the switch to if and de-indenting , I have not touched pgsql code. (I would not, not even with a ten foot barge pole.)
Comment #18
webchickBefore applying #15, I was getting a metric crap-load of errors on admin/content/comment/settings, which it turns out was from Anonymous user being set to 2.
Confirmed that this patch fixed the problem.
Comment #19
webchickOne weird thing. This causes uid 1's "Member for" to be 37 years 36 weeks. Without this patch, uid 2 (anon) gets 37 years 36 weeks. So it seems the 2nd insert statement is buggy.
Comment #20
chx CreditAttribution: chx commentedComment #21
David Strauss@webchick Right. The original INSERTs need to set "created" to time(). Here's my (slight) update. I've also made the serialization not assume the underlying PHP serialization format.
Comment #22
David StraussThis time, an attachment!
Comment #23
chx CreditAttribution: chx commentedAh yes, I wanted to do that but forgot. Good work.
Comment #24
webchickTagging this as a "Beta-Breaker" .. we definitely can NOT release a beta with this broken.
Comment #25
webchickTagging this as a "Beta-Breaker" .. we definitely can NOT release a beta with this broken.
Comment #26
Gábor HojtsyThe inserts and the update need more docs. It looks awkward. Let's start document the reasons to use the INSERTs and the UPDATE before the inserts. Document why we do the inserts and the update and then the data replacement. Otherwise we might not have a better option, so I think we can go with this.
Comment #27
Gábor HojtsyAlso: why don't we use real numbers in the INSERTs? If there are good reasons, we need to document them. AFAIS, if we use the real numbers (0, 1), we still get the same result, as with the INSERTs and UPDATE.
Comment #28
chx CreditAttribution: chx commentedMany, many comments added.
Comment #29
Gábor HojtsyThis is a complicated issue, so thanks for commenting the code. Unfortunately this is not an elegant solution, but the problem is awkward to start with. Committed.
Comment #30
David Strauss"Inserting uid 0 here confuses MySQL -- the next user might be created as uid 2 which is not what we want."
More specifically, MySQL interprets INSERTing uid 0 as a request to assign uid's value from the current auto_increment value. If you insert uid 0, the next user will be created as uid 2 in MySQL. The only room for a "maybe" is a bug in MySQL.
Comment #31
(not verified) CreditAttribution: commented