Posted by Heine on August 18, 2010 at 5:37pm
10 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | openid.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Berdir |
| Status: | closed (fixed) |
| Issue tags: | Security Advisory follow-up |
Issue Summary
This is a slightly restructured version of the patch that went into Drupal 6. All in one as the changes touch the same code.
Code by Damien, Christian (c96...) and myself.
I've test failures in the openid tests both with and without this patch on adding openid_identifier' => '@example*résumé;%25', to user 2.
I suppose this could use additional tests as well.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| openid-incomplete-verification-assertions-D7.patch | 13.03 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 22,949 pass(es). | View details |
Comments
#1
+ * The next series of updates should start at 8000.+ */
\ No newline at end of file
could the committer add a newline after these lines.
#2
While we are no in beta yet, openid_update_7000() should just be openid_update_6000(), and we can remove the db_table_exists() call.
#3
BTW this is a followup to SA-CORE-2010-002.
#4
Looks like nobody is working on this yet on the summit, I'll do a re-roll, doesn't look too complicated...
#5
Just a simple re-roll with the update function renamed and newlines added...
#6
+/**
+ * @defgroup openid-updates-6.x-to-7.x OpenID updates from 6.x to 7.x
+ * @{
+ */
^ Those should be in the Drupal 6 extra group.
#7
Obviously...
The test fail looks totally unrelated and works for me locally. Maybe the test is unstable....
#8
I looked at this patch and it looks RTBC, assuming that the testbot gives us the green light.
+++ modules/openid/openid.install@@ -84,3 +110,46 @@ function openid_requirements($phase) {
+ 'description' => 'The value of openid.response_nonce'
Very minor but we're missing a point in the description. I can fix that prior to committing.
#9
#10
The last submitted patch, openid-incomplete-verification-assertions-D7-3.patch, failed testing.
#11
#7: openid-incomplete-verification-assertions-D7-3.patch queued for re-testing.
#12
Hmm, the last 2 patches have failed for two different reasons within 2 hours. I haven't followed what has gone in today and/or if anything broke the test bot, but I requested a re-test and am crossing my fingers that this passes.
#13
This may be "critical," but it's not a beta-blocker. People barely notice when OpenID is broken in stable releases.
#14
$0.02: If I understood correctly, if this doesn't land before beta, we will have to change the update functions...
#15
This was rtbc before the bot glitch.
BTW Dries look at #8 before commiting ;)
#16
Great. Committed to CVS HEAD. Thanks.
#17
Automatically closed -- issue fixed for 2 weeks with no activity.