- Development - lists.drupal.org

[drupal-devel] [bug] UI allows longer role name than database
by Cvbge 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/18616 Post a follow up: http://drupal.org/project/comments/add/18616 Project: Drupal Version: 4.6.3 Component: database system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: plj Updated by: Cvbge Status: patch (code needs work) Attachment: http://drupal.org/files/issues/role.name_64.diff (2.61 KB) I have not tested mysql's update, but I think it should work. Cvbge Previous comments: ------------------------------------------------------------------------ Wed, 09 Mar 2005 13:38:12 +0000 : plj The Postgresql database script shipped with Drupal 4.5.2 has a 32 characters long varchar value for role name. The relevant UI form field, however, has attribute maxlength="64". This naturally causes an SQL error, if too long name is entered. The SQL part is still same in CVS version, but I don't know if the UI part has been fixed. Related SQL: CREATE TABLE role ( rid SERIAL, name varchar(32) NOT NULL default '', PRIMARY KEY (rid), UNIQUE (name) ); ------------------------------------------------------------------------ Tue, 28 Jun 2005 16:51:15 +0000 : plj Changing version to 4.6.1, and component to database system (perhaps more relevant than user system), as the problem still exists. It also seems that the length of the field in question is only 32 in both database.mysql and database.pgsql, even though the UI allows 64. Either UI or the database scripts should be fixed. ------------------------------------------------------------------------ Tue, 28 Jun 2005 17:06:18 +0000 : plj Attachment: http://drupal.org/files/issues/database_1.patch (660 bytes) On a second thought, here is a patch against both DBs. Both are untested, but the change is tiny. ------------------------------------------------------------------------ Sat, 06 Aug 2005 20:36:10 +0000 : Cvbge There should be also a patch for database/updates.inc ------------------------------------------------------------------------ Mon, 15 Aug 2005 11:20:09 +0000 : plj I've no idea how the update.inc's update numbering works. But it should excecute a statement like this: alter table role alter column name type varchar(64)

1 0

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

16 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: Jeremy(a)kerneltrap.org Status: patch (code needs review) 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. Jeremy(a)kerneltrap.org 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.

1 0

[drupal-devel] [bug] user/register returns user deny when a user is already logged in
by breyten 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/17664 Post a follow up: http://drupal.org/project/comments/add/17664 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: gufo1 Updated by: breyten -Status: patch (code needs review) +Status: patch (code needs work) I'd rather opt for Drupal to display a message along the lines of 'You can't register an account because you're already logged in.', instead of an equally confusing redirect. Secondly, I'd like to see consistency: If you're logged in now, and try to request a new password, Drupal will give you a 403 as well. breyten Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 15:46:56 +0000 : gufo1 Any user, even the administrator gets this user access error when he is already logged in. This causes confusion for users who are already logged in, instead it should redirect the user to the user page. ------------------------------------------------------------------------ Fri, 01 Jul 2005 18:55:33 +0000 : alexis Agree, I have the same problem with 4.6.2. ------------------------------------------------------------------------ Tue, 16 Aug 2005 11:28:48 +0000 : Tobias Maier Attachment: http://drupal.org/files/issues/user_register_return_to_userspage_user.modul… (1023 bytes) here comes a patch for this i needed relatively long to find out why this happens. the behavour was theoretically implemented but hook_menu() denied it...

1 0

[drupal-devel] [task] remove old updates
by chx 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/12381 Post a follow up: http://drupal.org/project/comments/add/12381 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: chx Status: patch (code needs review) /me feels stupid for his May reaction. Especially that when this issue was raised again on IRC, I was the first to mention the CVS repos... baaaah! So, I am fine with removing updates. chx Previous comments: ------------------------------------------------------------------------ Sun, 31 Oct 2004 22:48:40 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/updates_0.patch (72.62 KB) The attached patch removes the updates 80 through 109 from updates.inc and adds a "delete from cache" statement after updating. ------------------------------------------------------------------------ Sun, 31 Oct 2004 23:27:40 +0000 : TDobes big, big, big -1 on removing the old updates. Having these updates around makes life much easier for those upgrading from older versions and provides quick code samples on how to populate certain fields and tables for those migrating from other CMS'. I know, in particular, that the PHP code for the node_comment_statistics table and the thread field of the comments table have been infinitely valuable to me when navigating sites from other CMS'. I like the idea of clearing the cache after each run of update.php. There is a specific issue for it [1] as well. [1] http://drupal.org/node/12350 ------------------------------------------------------------------------ Sun, 13 Mar 2005 19:08:27 +0000 : chx tdobes is right there for I set this to "won't fix". ------------------------------------------------------------------------ Tue, 16 Aug 2005 13:44:26 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/updates_3.patch (69.64 KB) Don't recall why I wanted the delete from cache, but this patch removes all update prior to 4.5.0. Peopel who want inspiration from old updates still have plenty of them in the file and can look into our cvs repos.

1 0

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

16 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: walkah +Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx -Status: active +Status: patch (ready to be committed) 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. 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.

1 0

[drupal-devel] [task] remove old updates
by killes 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/12381 Post a follow up: http://drupal.org/project/comments/add/12381 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org -Status: won't fix +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/updates_3.patch (69.64 KB) Don't recall why I wanted the delete from cache, but this patch removes all update prior to 4.5.0. Peopel who want inspiration from old updates still have plenty of them in the file and can look into our cvs repos. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Sun, 31 Oct 2004 22:48:40 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/updates_0.patch (72.62 KB) The attached patch removes the updates 80 through 109 from updates.inc and adds a "delete from cache" statement after updating. ------------------------------------------------------------------------ Sun, 31 Oct 2004 23:27:40 +0000 : TDobes big, big, big -1 on removing the old updates. Having these updates around makes life much easier for those upgrading from older versions and provides quick code samples on how to populate certain fields and tables for those migrating from other CMS'. I know, in particular, that the PHP code for the node_comment_statistics table and the thread field of the comments table have been infinitely valuable to me when navigating sites from other CMS'. I like the idea of clearing the cache after each run of update.php. There is a specific issue for it [1] as well. [1] http://drupal.org/node/12350 ------------------------------------------------------------------------ Sun, 13 Mar 2005 19:08:27 +0000 : chx tdobes is right there for I set this to "won't fix".

1 0

[drupal-devel] [bug] user/register returns user deny when a user is already logged in
by Tobias Maier 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/17664 Post a follow up: http://drupal.org/project/comments/add/17664 Project: Drupal -Version: 4.5.0 +Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: gufo1 Updated by: Tobias Maier -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/user_register_return_to_userspage_user.modul… (1023 bytes) here comes a patch for this i needed relatively long to find out why this happens. the behavour was theoretically implemented but hook_menu() denied it... Tobias Maier Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 15:46:56 +0000 : gufo1 Any user, even the administrator gets this user access error when he is already logged in. This causes confusion for users who are already logged in, instead it should redirect the user to the user page. ------------------------------------------------------------------------ Fri, 01 Jul 2005 18:55:33 +0000 : alexis Agree, I have the same problem with 4.6.2.

1 0

[drupal-devel] [task] Remove old bugfix code
by drumm 16 Aug '05

16 Aug '05

Issue status update for http://drupal.org/node/28944 Post a follow up: http://drupal.org/project/comments/add/28944 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch (code needs review) Attachment: http://drupal.org/files/issues/update.php.diff (967 bytes) killes, who wrote the origional patch, said this code is okay to be removed. It was probably for the 4.4 -> 4.5 update. drumm

2 1

[drupal-devel] [feature] Patch: add ability to limit searches to given taxonomy(ies)
by jsaints 15 Aug '05

15 Aug '05

Issue status update for http://drupal.org/node/28933 Post a follow up: http://drupal.org/project/comments/add/28933 Project: Drupal Version: 4.6.0 Component: search.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jsaints Updated by: jsaints Status: patch (code needs review) Attachment: http://drupal.org/files/issues/search_by_taxonomy_patches.zip (3.09 KB) Here is a quick patch to the search.module and node.module That allows for site search to be limited by taxonomy. Taxonomy limited searches would be available via a URL like: 1) http://my.site.com/search/node/taxonomy/term/1+5+6/my+search+text 2) http://my.site.com/search/node/taxonomy/term/1,5,6/my+search+text These two searches would return respectivly: 1) nodes matching "my search text" in taxonomies 1 or 5 or 6 2) nodes matching "my search text" in taxonomies 1 and 5 and 6 This functionality will not interfere with the current search syntax http://my.site.com/search/node/my+search+text. which will continue to return all nodes that match the text "my search here". The patches are for Drupal version 4.6. If the community likes this functionality, I would like to commit it to CVS jsaints

1 1

[drupal-devel] [bug] drupal base and E_NOTICE
by Thomas Ilsche 15 Aug '05

15 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: Thomas Ilsche Status: patch (code needs review) > 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) Thomas Ilsche 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 ?

1 0