August 2005 - Development - lists.drupal.org

[drupal-devel] Users self-publishing their content
by Todd Grimason 18 Aug '05

18 Aug '05

Hi all- I asked about this on the support list first and have been looking into it, and wanted to see if you all could confirm my conclusions: I'm running a site(s) which is largely a group weblog (also some "stories"). The authors are trusted to not post nonsense or inappropriate material, but not trusted enough (mostly technically) to have full "admin nodes" privs where they can access each others' work. What they (undestandably IMO) want is the ability to save drafts, i.e. works in progress they don't yet want to publish. With the unmodified core Drupal codebase, I can have the default workflow be "publish" or not. And that works. They get once chance to write it, polish over and over, and hit "submit" -- it's published. Or I set the default to "not published", and they submit it, and then have no way to publish it and I (or another admin) has to publish it. So clearly (right?) I need to find another way to do this, it's not "out-of-the-box-able". A long while ago I tried using workflow.module to accomplish this, and had some issues I can't recall. So I hacked the hell out of a bunch of files, made a mess, and got what I wanted. But not in a resuable way. So this time I've tried to create a clean add-on module, using nodeapi hooks, and it *almost* works cleanly. I can easily get the 'Published' checkbox to appear correctly when the author has the appropriate permissions (in my lil' module), *but* node.module#validate resets the node->status flag on every form interaction. So at this point I stuck in: // "publish my content" is a priv defined in my module if (!user_access("publish my content")) { $node->status = in_array('status', $node_options); } around line 1238 or so in node.module, 4.6.2. Finally, my question: is there any way to do this cleanly -- without modifying core Drupal code? (as currently publishing appears to be an all-or-nothing situation). thanks in advance! === PS: my understanding is workflow.module is in flux right now, with issues with revisions. So I'd guess it won't be ready until post-4.7, but of course I need this sooner - like a week or two. -- ________________________________ toddgrimason*todd[ at ]slack.net

2 2

[drupal-devel] [bug] drupal base and E_NOTICE
by hlslaughter 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/28540 Post a follow up: http://drupal.org/project/comments/add/28540 Project: Drupal Version: 4.6.0 Component: base system Category: bug reports Priority: minor Assigned to: Anonymous Reported by: hlslaughter Updated by: hlslaughter Status: patch (code needs review) i noticed that some folks are silencing errors instead of doing long-hand 1) not ideal makes "E_ALL" impractical: $foo = $baz["val"]; // generates notice if "val" is not set 2) cleaner: if ( isset($baz["val"]) ) { $foo = $baz["val"]; } // or my preference: $foo = isset($baz["val"]) ? $baz["val"] : NULL; 3) alt way: $foo = @$baz["val"]; methods 1 and 2 can become very ugly and hard to read if you're dealing with long/nested arrays. i like method 3 as it's easy to read and the silencer/@ indicates that the author is aware this syntax may generate a warning notice, but it's ok. what do you guys think? hlslaughter Previous comments: ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:19:33 +0000 : hlslaughter ok, instead of whining in the forums, i'll submit a ticket. i'm sure i'm not the first person to bring this up, and i apologize in advance if this issue is already decided/closed. but my issue is this: would it be possible to review the core modules and clean up code to the point that it would be feasible to run drupal with notices turned on (at least in a dev environment)? i think it would be pretty trivial to clean most/all of these errors from the code base. the cost of this effort would be relatively teeny, and the benefits would be great. admins would no longer panic if they happen to see these messages; with an option added to the settings screen, developers could turn on notices and would write cleaner code, which would probably result in fewer bugs in 3rd party modules; etc... you know the routine 8*) after turning on E_NOTICE in common.inc, i see the following notice messages when accessing the homepage of my site: occurences - message ----V--------------V-------- 12 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 114. 10 notice: Undefined variable: teaser in /var/www/html/modules/opt/attachment.module on line 37. 6 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 120. 6 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 176. 4 notice: Undefined variable: rows in /var/www/html/modules/opt/attachment.module on line 352. 4 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 123. 4 notice: Undefined variable: attachments in /var/www/html/modules/opt/attachment.module on line 279. 1 notice: Undefined variable: node in /var/www/html/modules/node.module on line 1950. 1 notice: Undefined offset: 0 in /var/www/html/includes/pager.inc on line 59. 1 notice: Undefined index: op in /var/www/html/modules/node.module on line 1636. 1 notice: Undefined index: node.taxonomy in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: node.picture in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.title in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.site_slogan in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.mission in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.help in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: from in /var/www/html/includes/pager.inc on line 53. 1 notice: Undefined index: footer.message in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: footer.blocks in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: edit in /var/www/html/modules/node.module on line 1637. 1 notice: Undefined index: 2 in /var/www/html/modules/filter.module on line 629. 1 notice: Undefined index: in /var/www/html/includes/common.inc on line 506. 1 notice: Trying to get property of non-object in /var/www/html/modules/node.module on line 1950. again, no offense, and sorry if i'm trying to bring a dead horse back to life. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:22:44 +0000 : killes(a)www.drop.org This is being worked on for the next Drupal version. Please install yourself a test site on cvs and submit individual patches. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:32:57 +0000 : hlslaughter thanks, i'll do this. i take it patches are just posted here? do i just post a diff -N, title it patch and mark the component appropriately? ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:39:08 +0000 : killes(a)www.drop.org Yes, attach here or create new issue. diff -up is what we usually use. Thanks. ------------------------------------------------------------------------ Tue, 09 Aug 2005 20:24:16 +0000 : hlslaughter Attachment: http://drupal.org/files/issues/test.patch (25.21 KB) attached is results of: harry@penny:~/cvs/third_party/drupal/drupal> diff -Naurp . /var/www/html/drupal_cvs/ > ../test.patch ok, would this type of patch be suitable? it includes fixes for all the E_NOTICE messages generated when accessing the homepage of a new, unmodified drupal installation straight from CVS (as of Aug 9 13:14) ------------------------------------------------------------------------ Tue, 09 Aug 2005 21:01:37 +0000 : killes(a)www.drop.org You are almost there. How to best prepare patches is to run Drupal directly from a cvs checkout. then you can fix the checkout and then run cvs diff -up This way the unrelated files in the patch won't be included. ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:29:12 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/node.user.e_notice.path.txt (1.79 KB) I have a small patch for node.module and user.module that fixes 3 notices in the current HEAD. Please especially have a look at node_access_view_all_nodes, I simply do not get the current use of $node->nid (do I need some more sleep or is this really bogus?) ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:36:22 +0000 : Thomas Ilsche About the newlines at eof: Sorry didn't notcied this as phped removed them manually, is there any reason for them, I got them in many drupal files? Maybe worth a fix aswell? ------------------------------------------------------------------------ Sun, 14 Aug 2005 15:12:00 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/common.inc.enotice.patch.txt (668 bytes) Another patch for common inc. ------------------------------------------------------------------------ Mon, 15 Aug 2005 17:30:34 +0000 : hlslaughter just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). also, sorry i haven't submitted patches, i guess i'm not confident i understand the correct method to submit. is it as follows? 1) cd $drupal_cvs/somedir/ (optionally: cvs update .) 2) edit some.file 3) cvs diff -up > some.file.patch 4) attach some.file.patch to appropriate thread at drupal.org 5) assumming patch is good and correct, pray that someone notices the patch and applies it within CVS ? ------------------------------------------------------------------------ Mon, 15 Aug 2005 18:25:15 +0000 : Thomas Ilsche > just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). I wanted to keep it semantically equivalent to if($var). isset is not. This might be especially critical against boolean values. The very problem is the fact that many properties of objects / indexes of arrays are not initialized. !empty is simply on the safe side. I think with the patches it is important to do them from the main drupal dir and not subdirs (but I'm not sure either, a faq/tut/introduction would be cool)

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by breyten 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Jeremy(a)kerneltrap.org Updated by: breyten Status: patch (code needs review) Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it? breyten Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security.

1 0

[drupal-devel] [task] Make xmlrpc_multicall working
by chx 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/28826 Post a follow up: http://drupal.org/project/comments/add/28826 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/xmlrpc_multicall.patch (2.77 KB) As you can see from the amount of code modified, xmlrpc_multicall was almost ready when the new library got into the core. This is the patch that deletes "almost" :) Also, some commented out debug code is removed. chx

2 3

[drupal-devel] [bug] Upgrade from 4.4 to 4.5.1 creates incorrect columns in menu table when using PostgreSQL
by Dries 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/14928 Post a follow up: http://drupal.org/project/comments/add/14928 Project: Drupal Version: 4.5.5 Component: postgresql database Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Anonymous Updated by: Dries -Status: patch (ready to be committed) +Status: patch (code needs work) Committed to DRUPAL-4-5 and DRUPAL-4-6. Patch failed against HEAD. Setting this to 'code needs work'. Dries Previous comments: ------------------------------------------------------------------------ Thu, 30 Dec 2004 05:30:15 +0000 : Anonymous Attachment: http://drupal.org/files/issues/updates.inc_2.patch (956 bytes) I just upgraded my installation from 4.4 to 4.5.1 and got these errors when trying to administer the image module: pg_query(): query failed: ERROR: invalid input syntax for integer They were all for the menu table. On investigation, i found this in database/updates.inc: if ($GLOBALS['db_type'] == 'mysql') { $ret[] = update_sql("ALTER TABLE {menu} ADD description varchar(255) DEFAULT '' NOT NULL"); } else { $ret[] = update_sql("ALTER TABLE {menu} ADD description smallint"); $ret[] = update_sql("ALTER TABLE {menu} ALTER COLUMN description SET DEFAULT '0'"); $ret[] = update_sql("UPDATE {menu} SET description = '0'"); $ret[] = update_sql("ALTER TABLE {menu} ALTER COLUMN description SET NOT NULL"); } WTH??? How is a smallint on other databases equivalent to varchar on mysql? Attached is a patch to fix the updates.inc file. ------------------------------------------------------------------------ Tue, 09 Aug 2005 02:17:33 +0000 : gordon should be marked as patch ------------------------------------------------------------------------ Tue, 16 Aug 2005 17:40:23 +0000 : Cvbge Attachment: http://drupal.org/files/issues/updates.inc-4.5.diff (1 KB) True, updated patch for latest 4-5 (although now it's a bit late for 4.5...) ------------------------------------------------------------------------ Tue, 16 Aug 2005 17:44:43 +0000 : Cvbge The patch also applies to 4-6 and HEAD, please apply.

1 0

[drupal-devel] [feature] Clean up and document user_roles() function
by mathias 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/29064 Post a follow up: http://drupal.org/project/comments/add/29064 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: minor Assigned to: mathias Reported by: mathias Updated by: mathias Status: patch (code needs review) Attachment: http://drupal.org/files/issues/user_roles.patch (978 bytes) Documented user_roles() and set reasonable defaults for the function parameters. mathias

1 0

[drupal-devel] [task] Rewrite to use procedures and conditionally include xmlrpc.inc
by chx 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/7458 Post a follow up: http://drupal.org/project/comments/add/7458 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/xmlrpc_comment_spaces.patch (2.17 KB) Dries commited this but something went foobar in the latest two patches and now my phpdoc is broken :( only changes are in the comment. chx Previous comments: ------------------------------------------------------------------------ Thu, 29 Apr 2004 18:15:33 +0000 : killes(a)www.drop.org Drupal includes xmlrpc.inc for every non-cached page view. However, the functions from the file are only rarely used. Therefore we need a patch that includes the file from the functions that want to use it. ------------------------------------------------------------------------ Sun, 02 May 2004 18:30:55 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc.patch (4.64 KB) Here is a patch that moves the include statement to the functiosn where xmlrpc.inc is actually needed. The patch coul dneed some testing as I do not use those modules (ping, bloggerapi) myself. ------------------------------------------------------------------------ Mon, 03 May 2004 21:34:39 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc_0.patch (6.38 KB) Patch updated, I had forgotten drupal.module. thanks to crschmidt for spotting this. ------------------------------------------------------------------------ Mon, 03 May 2004 22:17:52 +0000 : Steven drupal_xml_parser_create() does not require xmlrpc.inc... ------------------------------------------------------------------------ Mon, 03 May 2004 22:27:55 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc_1.patch (6.06 KB) Ok, updated. ------------------------------------------------------------------------ Mon, 03 May 2004 22:41:53 +0000 : Dries If xmlrpc.php includes xmlrpc.inc, why do we have to include it in blogapi module's functions once more? Also, the inclusion of xmlrpc.inc in drupal.module can probably be avoided by including xmlrpc.inc in cron.php? Am I missing something? ------------------------------------------------------------------------ Mon, 03 May 2004 22:49:58 +0000 : crschmidt Not all of the tasks in drupal.module are run with cron.php: for example, the distributed authentication comes in through drupal.module, which requires code from xmlrpc.inc to create the messages it sends out, as far as I can tell. So, the drupal.module needs it for distributed authentication stuff, not just for pinging servers and so on. ------------------------------------------------------------------------ Mon, 03 May 2004 23:38:35 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc_2.patch (2.8 KB) "If xmlrpc.php includes xmlrpc.inc, why do we have to include it in blogapi module's functions once more?" As I said, I am not that familiar with that part of Drupal, did not investigate under whch circumstances blogapi functions could be called, and wanted to stay on the safe side. If you are sure that blogapi is always called from xmlrpcs.php, then apply the appended version. ------------------------------------------------------------------------ Tue, 04 May 2004 05:40:48 +0000 : Dries I'm not 100% sure either, I was merely suggesting an alternative. I think the second patch might work tough, in which case it would be preferred for its simplicity. Not? I'll add "Testing Gerhard's blog API patch to my short term TODO list". Hopefully I get to it tonight after work. Thanks Gerhard. ------------------------------------------------------------------------ Wed, 12 May 2004 20:12:10 +0000 : Kjartan Good patch, just a few minor comments. - Use single quotes for constant strings. - I noticed some indentations that were off the 2 column, looks like the are in the current code though. - drupal_directory_ping() will only be invoked from xmlrpc.php so there shouldn't be a need to include the file there. ------------------------------------------------------------------------ Wed, 12 May 2004 22:40:06 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc_2_0.patch (2.64 KB) Updated. Don't know to which spaces you are referring. Guess Drupal CVS needs to be code checked again. ------------------------------------------------------------------------ Wed, 04 Aug 2004 01:39:34 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmlrpc_3.patch (2.81 KB) Updated again. It appears that parts of the old patch made it into drupal.module, but the include statement was still in common.inc. ------------------------------------------------------------------------ Wed, 04 Aug 2004 05:47:45 +0000 : TDobes +1... I tested the patch with blogapi.module and w.bloggar and it works fine, as far as I can tell. I like any patch that includes less code on each page view. ------------------------------------------------------------------------ Wed, 04 Aug 2004 20:36:43 +0000 : Dries Committed to HEAD. Thanks. ------------------------------------------------------------------------ Fri, 13 Aug 2004 18:47:36 +0000 : walkah Attachment: http://drupal.org/files/issues/xmlrpc-include.diff (2.38 KB) this patch didn't break blogapi.module, it's true. but it _did_ break drupal.module's distributed authentication feature. the problem is that xmlrpc.inc declares some classes, which will not work if the include statement appears inside a function. see the following for more details: http://us4.php.net/manual/en/language.functions.php (specifically the comment by albaity at php4web dot com) the attached patch restores the drupal.module functionality (as well as reintroduces the include in common.inc). I'm afraid that's just the way it has to be, sorry killes ;) (this patch also fixes a typo in drupal.module where the 'name' array key was not quoted - which may break things under higher error reporting levels). ------------------------------------------------------------------------ Fri, 13 Aug 2004 19:23:28 +0000 : Anonymous Can't you include common.inc from drupal.module if it needs that file? locale.module also includes its locale.inc without punishing the user who doesn't care about its functionality. Gerhard ------------------------------------------------------------------------ Sat, 14 Aug 2004 06:29:36 +0000 : Dries Including the XML-RPC library at the top of those files/modules that need it sounds like a fair alternative. ------------------------------------------------------------------------ Sat, 14 Aug 2004 11:19:50 +0000 : Steven But that doesn't really help unless you have no XML-RPC modules enabled at all... ------------------------------------------------------------------------ Sat, 14 Aug 2004 12:58:52 +0000 : JonBob What is more, I don't think it even fixes the bug. The problem is that class declarations in PHP4 are not guaranteed to work correctly when made inside a function. This applies to declarations immediately inside a function definition, or in files included from inside a function definition, or even from files included from files included from function definitions. Note that since module files are included from module_load(), this means class definitions or includes of files that have them cannot safely be in modules. Basically, conditional inclusion of classes is a no-no. To get around this we'd have to rewrite xmlrpc.inc using functions rather than object methods. ------------------------------------------------------------------------ Mon, 16 Aug 2004 18:29:40 +0000 : Dries I reverted the patch. ------------------------------------------------------------------------ Tue, 17 Aug 2004 15:39:25 +0000 : walkah ya, but you forgot to put the include_once back in common.inc . :) ------------------------------------------------------------------------ Tue, 17 Aug 2004 17:59:05 +0000 : killes(a)www.drop.org Can't just somebody step forward and rewrite that class in functions? ------------------------------------------------------------------------ Tue, 31 Aug 2004 21:41:24 +0000 : killes(a)www.drop.org I'd like to keep this open. It is so nicely assigned. ;-> ------------------------------------------------------------------------ Wed, 01 Sep 2004 12:35:31 +0000 : Anonymous *grumble* ------------------------------------------------------------------------ Mon, 31 Jan 2005 23:05:37 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/xmplrpc.patch (695 bytes) Here is an alternative approach. XML-RPC requests are suppoed to be POST requests, not GET. By jvandyk and myself. ------------------------------------------------------------------------ Mon, 31 Jan 2005 23:22:18 +0000 : jvandyk This patch affects the inclusion of xmlrpc, which is the xmlrpc client, not xmlrpcs, which is the server and subject to the "all XML-RPC requests are POST requests" rule. Thus, it is ineffective. However, we should consider eliminating the inclusion of xmlrpc.inc at all in common.inc. This would speed up Drupal at the expense of modules that use XML-RPC calls having to include xmlrpc.inc themselves. grepping for xmlrpc_ finds occurrences in drupal.module for cross-drupal-site authentication, foaf.module, livejournalmodule, and waypath.module. ------------------------------------------------------------------------ Mon, 31 Jan 2005 23:24:23 +0000 : jvandyk And now that I've revisited JonBob's comments on this, I can see why it's currently the way it is and can't be conditionally included. Blech. ------------------------------------------------------------------------ Tue, 01 Feb 2005 03:51:54 +0000 : tangent It seems that implementing xmlrpc as a (required) module would work around the include issue. I'm working on a xmlrpc.module from scratch, sort of as a Drupal learning exercise, which uses the XML-RPC extension functions instead of the xmlrpc.inc that we're using now and will reduce the code to a few lines instead of the big class that exists now. Before I get too far into it though, I'm interested if anyone may have thoughts about why a module is not the way to go, or if there is some reason we shouldn't be using the XML-RPC functions. ------------------------------------------------------------------------ Tue, 16 Aug 2005 13:46:41 +0000 : chx Attachment: http://drupal.org/files/issues/xmlrpc_cond.patch (4.4 KB) The new library lets this happen. Also, this patch unifies the xmlrpc() and xmlrpc_multicall() calls. Most sites now load 12K less PHP code for almost all page requests. ------------------------------------------------------------------------ Tue, 16 Aug 2005 17:51:14 +0000 : chx Attachment: http://drupal.org/files/issues/xmlrpc_cond_0.patch (4.58 KB) Dries asked for more clear phpdoc. Robert Douglass and I have enhanced it. ------------------------------------------------------------------------ Tue, 16 Aug 2005 18:01:45 +0000 : chx Attachment: http://drupal.org/files/issues/xmlrpc_cond_1.patch (4.69 KB) Steven found that the faster ./ is missing from xmlrpc.php

1 0

[drupal-devel] [bug] Statistic information about node reads is shown to unprivileged users
by stefan nagtegaal 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/22565 Post a follow up: http://drupal.org/project/comments/add/22565 Project: Drupal Version: cvs Component: statistics.module Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: massabob Updated by: stefan nagtegaal Status: patch (ready to be committed) Very nice patch! One option less... Works flawelessly on my localhost... :-) stefan nagtegaal Previous comments: ------------------------------------------------------------------------ Wed, 11 May 2005 07:51:40 +0000 : massabob Information about '%count reads' in node's footer is shown to unprivileged users. I suggest that function statistics_link in statistics.module should be corrected in this way: // Original function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0)) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } // Fixed function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0) && user_access('display statistics')) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } The only change is in "&& user_access('display statistics')" on line 98. ------------------------------------------------------------------------ Fri, 27 May 2005 01:44:25 +0000 : RobRoy Should read user_access('access statistics') not user_access('display statistics'). ------------------------------------------------------------------------ Fri, 27 May 2005 11:19:36 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/user.access.stats.patch (730 bytes) And here that is in patch form. Robin ------------------------------------------------------------------------ Wed, 01 Jun 2005 04:09:16 +0000 : Steven I'm not sure about this patch: often, read counts are shown directly on the site. But if the permission for viewing the counts is the same as the permission for accessing the administrator's detailed logs, then you wouldn't give that to everyone. There is already an option to choose whether counts are displayed. Perhaps we could change that to "No" "For priviledged users" "For everyone". In last case it acts like it is now, it the second case it requires "access statistics" permission. What do you think? ------------------------------------------------------------------------ Wed, 01 Jun 2005 14:13:54 +0000 : Robin Monks Sounds good to me. I'll try to code something up for this. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 13:55:49 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats.patch (2.26 KB) Here is the patch. Uses a switch to choose between signed in users, all users, users with permissions or noone. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 14:08:16 +0000 : Robin Monks I tested this patch with various settings on my local install and it worked fine. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:30:25 +0000 : Bèr Kessels Is there a reason why you check for $user->uid? Whaen someone has "access statistics" set to anonymous users, your check for $user->uid will override taht settings. Not good IMO. <?php $group .= form_radios(t('Display counter values'), 'statistics_display_counter', variable_get('statistics_display_counter', 0), array('1' => t('For all users'), '2' => t('For authenticated users'), '3' => t('For priviledged users'), '0' => t('Disabled')), t('Display how many times given content has been viewed.')); ?> is very inconsistent. please use *only* the permissions page to set permissions, and do not create new permissions-alike settings in any configuration pages. I would say a simple check for user_access('access statistics') will do the trick ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:32:53 +0000 : Bèr Kessels sorry, i meant to say user_access('access statistics counter'), not user_access('access statistics'). We already have "access statistics'" an additional "access statistics counter" for showing users the counter should work Ber ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:49:48 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/statistics.module (23.1 KB) OK, here is a patch to that end... Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:00:50 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_0.patch (1 KB) Um, let's just pretend I didn't just upload the entire stats module. OK? OK! Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:05:59 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_1.patch (1.06 KB) Hotfix. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:11:23 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_2.patch (1.11 KB) Hopefuly the final version. Thanks to chx and berkes for pulling it apart ;-) Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:21:38 +0000 : Bèr Kessels +1 for this patch. it adds functionality, but does not add clutter nor any config options. ------------------------------------------------------------------------ Fri, 10 Jun 2005 02:39:06 +0000 : frjo +1 I have implemented the patch authstats_2.patch on my site running Drupal 4.6.1 and it works well. ------------------------------------------------------------------------ Fri, 01 Jul 2005 08:10:22 +0000 : mfb +1 ------------------------------------------------------------------------ Fri, 01 Jul 2005 12:03:56 +0000 : Jose A Reyero +1 I tested the patch, works fine, and I'm always for "more options" ;-) ------------------------------------------------------------------------ Mon, 01 Aug 2005 21:17:48 +0000 : Junyor +1. Using this on my site now. ------------------------------------------------------------------------ Tue, 02 Aug 2005 21:53:15 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/node_counter_permission.patch (2.97 KB) This patch revision removes the duplicate show counter option from the settings. Thanks UnConeD! Robin ------------------------------------------------------------------------ Wed, 03 Aug 2005 15:44:59 +0000 : Dries -1, the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name. Is this different from the 'Count content views' setting? ------------------------------------------------------------------------ Wed, 03 Aug 2005 20:57:39 +0000 : Robin Monks /the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name. / Suggestions: view node access counter view node view counter access node view counter view node hit counter I rather like "view node access counter". Dries, would this satisify you? /Is this different from the 'Count content views' setting?/ Yes, on two counts. * These permissions are for users, not for admins. The count content views setting changes both * This setting lets the administrator choose specific groups to see the counter (eg, moderators, or only registered users) Robin ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:56:40 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/stat_counter_permission.HEAD.patch (3.04 KB) New version of patch that renames the permission to "view node access counter". Tested on latest HEAD. Ready to commit! Robin

1 0

[drupal-devel] [bug] Statistic information about node reads is shown to unprivileged users
by Robin Monks 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/22565 Post a follow up: http://drupal.org/project/comments/add/22565 Project: Drupal Version: cvs Component: statistics.module Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: massabob Updated by: Robin Monks Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/stat_counter_permission.HEAD.patch (3.04 KB) New version of patch that renames the permission to "view node access counter". Tested on latest HEAD. Ready to commit! Robin Robin Monks Previous comments: ------------------------------------------------------------------------ Wed, 11 May 2005 07:51:40 +0000 : massabob Information about '%count reads' in node's footer is shown to unprivileged users. I suggest that function statistics_link in statistics.module should be corrected in this way: // Original function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0)) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } // Fixed function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0) && user_access('display statistics')) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } The only change is in "&& user_access('display statistics')" on line 98. ------------------------------------------------------------------------ Fri, 27 May 2005 01:44:25 +0000 : RobRoy Should read user_access('access statistics') not user_access('display statistics'). ------------------------------------------------------------------------ Fri, 27 May 2005 11:19:36 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/user.access.stats.patch (730 bytes) And here that is in patch form. Robin ------------------------------------------------------------------------ Wed, 01 Jun 2005 04:09:16 +0000 : Steven I'm not sure about this patch: often, read counts are shown directly on the site. But if the permission for viewing the counts is the same as the permission for accessing the administrator's detailed logs, then you wouldn't give that to everyone. There is already an option to choose whether counts are displayed. Perhaps we could change that to "No" "For priviledged users" "For everyone". In last case it acts like it is now, it the second case it requires "access statistics" permission. What do you think? ------------------------------------------------------------------------ Wed, 01 Jun 2005 14:13:54 +0000 : Robin Monks Sounds good to me. I'll try to code something up for this. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 13:55:49 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats.patch (2.26 KB) Here is the patch. Uses a switch to choose between signed in users, all users, users with permissions or noone. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 14:08:16 +0000 : Robin Monks I tested this patch with various settings on my local install and it worked fine. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:30:25 +0000 : Bèr Kessels Is there a reason why you check for $user->uid? Whaen someone has "access statistics" set to anonymous users, your check for $user->uid will override taht settings. Not good IMO. <?php $group .= form_radios(t('Display counter values'), 'statistics_display_counter', variable_get('statistics_display_counter', 0), array('1' => t('For all users'), '2' => t('For authenticated users'), '3' => t('For priviledged users'), '0' => t('Disabled')), t('Display how many times given content has been viewed.')); ?> is very inconsistent. please use *only* the permissions page to set permissions, and do not create new permissions-alike settings in any configuration pages. I would say a simple check for user_access('access statistics') will do the trick ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:32:53 +0000 : Bèr Kessels sorry, i meant to say user_access('access statistics counter'), not user_access('access statistics'). We already have "access statistics'" an additional "access statistics counter" for showing users the counter should work Ber ------------------------------------------------------------------------ Thu, 09 Jun 2005 16:49:48 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/statistics.module (23.1 KB) OK, here is a patch to that end... Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:00:50 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_0.patch (1 KB) Um, let's just pretend I didn't just upload the entire stats module. OK? OK! Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:05:59 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_1.patch (1.06 KB) Hotfix. Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:11:23 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/authstats_2.patch (1.11 KB) Hopefuly the final version. Thanks to chx and berkes for pulling it apart ;-) Robin ------------------------------------------------------------------------ Thu, 09 Jun 2005 17:21:38 +0000 : Bèr Kessels +1 for this patch. it adds functionality, but does not add clutter nor any config options. ------------------------------------------------------------------------ Fri, 10 Jun 2005 02:39:06 +0000 : frjo +1 I have implemented the patch authstats_2.patch on my site running Drupal 4.6.1 and it works well. ------------------------------------------------------------------------ Fri, 01 Jul 2005 08:10:22 +0000 : mfb +1 ------------------------------------------------------------------------ Fri, 01 Jul 2005 12:03:56 +0000 : Jose A Reyero +1 I tested the patch, works fine, and I'm always for "more options" ;-) ------------------------------------------------------------------------ Mon, 01 Aug 2005 21:17:48 +0000 : Junyor +1. Using this on my site now. ------------------------------------------------------------------------ Tue, 02 Aug 2005 21:53:15 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/node_counter_permission.patch (2.97 KB) This patch revision removes the duplicate show counter option from the settings. Thanks UnConeD! Robin ------------------------------------------------------------------------ Wed, 03 Aug 2005 15:44:59 +0000 : Dries -1, the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name. Is this different from the 'Count content views' setting? ------------------------------------------------------------------------ Wed, 03 Aug 2005 20:57:39 +0000 : Robin Monks /the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name. / Suggestions: view node access counter view node view counter access node view counter view node hit counter I rather like "view node access counter". Dries, would this satisify you? /Is this different from the 'Count content views' setting?/ Yes, on two counts. * These permissions are for users, not for admins. The count content views setting changes both * This setting lets the administrator choose specific groups to see the counter (eg, moderators, or only registered users) Robin

1 0

[drupal-devel] [feature] Restrict archives by node type
by eafarris 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/29057 Post a follow up: http://drupal.org/project/comments/add/29057 Project: Drupal Version: cvs Component: archive.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: eafarris Updated by: eafarris Status: patch (code needs review) Attachment: http://drupal.org/files/issues/archive-restrictions.patch (2.55 KB) I am assisting in the rollout of a new site, and we wanted the ability to choose which content types caused a hit on the archive calendar, and the archive pages. Attached is a patch against Drupal HEAD implementing this feature. eafarris

1 0