- Development - lists.drupal.org

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

10 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) 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. 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

1 0

[drupal-devel] [feature] I added few error messages
by kubaZygmunt 10 Aug '05

10 Aug '05

Issue status update for http://drupal.org/node/27695 Post a follow up: http://drupal.org/project/comments/add/27695 Project: Drupal Version: cvs Component: taxonomy.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: kubaZygmunt Updated by: kubaZygmunt Status: patch Attachment: http://drupal.org/files/issues/taxonomy_3.module (52.92 KB) During writing my test for Drupaltest I saw that we don't have error messages in taxonomy module, so you can add taxonomy with empty name!! :) I've added something which prevent from this kubaZygmunt

4 6

[drupal-devel] [bug] drupal base and E_NOTICE
by killes 09 Aug '05

09 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: killes(a)www.drop.org -Status: fixed +Status: patch (code needs work) 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. killes(a)www.drop.org 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)

1 0

[drupal-devel] [bug] make standard roles translatable
by Cvbge 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/5570 Post a follow up: http://drupal.org/project/comments/add/5570 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: stefan nagtegaal Updated by: Cvbge -Status: patch (code needs review) +Status: patch (code needs work) Ok, this won't work, because roles are selected from db when put on admin/access. The only way I see to solve this (although it's still not clean) is to a) make default roles editable (so user can change the 'anonymous user' to something else), but not deletable b) make sure anon user and auth user have static id c) do not check in code for name, but for role id Need to think about it... Cvbge Previous comments: ------------------------------------------------------------------------ Sun, 01 Feb 2004 18:03:37 +0000 : stefan nagtegaal There are several things that would need to be fixed before we are going to release drupal 4.4. One of that is proper translation.. - The basic roles 'Anonymous user'/'Anonymous' & 'Authenticated user' aren't translatable on the following places (admin >> users / admin >> users >> roles / admin >> users >> permission / ) ------------------------------------------------------------------------ Fri, 23 Jul 2004 17:19:35 +0000 : killes(a)www.drop.org Changing title. ------------------------------------------------------------------------ Wed, 06 Oct 2004 09:53:48 +0000 : stefan nagtegaal This is still a problem.. The default roles 'Anonymous user' and 'Authenticated user' can not be translated. ------------------------------------------------------------------------ Wed, 06 Oct 2004 12:55:38 +0000 : Axel But why this bug is critical? Drupal works on such manner long time. This bug don't prevent Drupal to work. I think priority of this bug must be set as 'normal', but not 'critical'. ------------------------------------------------------------------------ Wed, 03 Nov 2004 18:53:46 +0000 : com2 I would say either committed to multilingual / localization or not. Calling it a feature seems to say "translation has a low priority". When that is the case, why all the trouble to localize? Jus drop localization and make life easier! All depends on the chosen concept of Drupal. If you choose Drupal to be multilingual / localized, then this is definitely is a bug. You cannot expect a non-english speaker to understand these special users. What happens when I just translate them in table /role/? Would it break Drupal? ------------------------------------------------------------------------ Wed, 04 May 2005 13:48:34 +0000 : schalker As I can tell, standard roles still can not be translated. Is this correct? Is there a work-around or some plan to implement this? I'd agree with the former poster com2 that this is more a bug than a feature request: I love the locale module, but I cannot see a reason why almost everything is translateable, but not the standard roles. If there is no reason for this behaviour /from a user-point-of-view/, then this is a bug, I'd say. ------------------------------------------------------------------------ Wed, 04 May 2005 14:05:33 +0000 : schalker Sorry, it's me again. I searched on and on to find a similar bug report, but I haven't found one. So as two people think this is a bug, I change the category of this issue back to "bug report". ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:40:42 +0000 : Cvbge Attachment: http://drupal.org/files/issues/translate_default_roles.diff (1.84 KB) Patch not tested, but it's quite simple. I'd like to hear some thoughts about t()ing default roles (anonymous and authenticated user).

1 0

[drupal-devel] [bug] make standard roles translatable
by Cvbge 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/5570 Post a follow up: http://drupal.org/project/comments/add/5570 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: stefan nagtegaal Updated by: Cvbge -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/translate_default_roles.diff (1.84 KB) Patch not tested, but it's quite simple. I'd like to hear some thoughts about t()ing default roles (anonymous and authenticated user). Cvbge Previous comments: ------------------------------------------------------------------------ Sun, 01 Feb 2004 18:03:37 +0000 : stefan nagtegaal There are several things that would need to be fixed before we are going to release drupal 4.4. One of that is proper translation.. - The basic roles 'Anonymous user'/'Anonymous' & 'Authenticated user' aren't translatable on the following places (admin >> users / admin >> users >> roles / admin >> users >> permission / ) ------------------------------------------------------------------------ Fri, 23 Jul 2004 17:19:35 +0000 : killes(a)www.drop.org Changing title. ------------------------------------------------------------------------ Wed, 06 Oct 2004 09:53:48 +0000 : stefan nagtegaal This is still a problem.. The default roles 'Anonymous user' and 'Authenticated user' can not be translated. ------------------------------------------------------------------------ Wed, 06 Oct 2004 12:55:38 +0000 : Axel But why this bug is critical? Drupal works on such manner long time. This bug don't prevent Drupal to work. I think priority of this bug must be set as 'normal', but not 'critical'. ------------------------------------------------------------------------ Wed, 03 Nov 2004 18:53:46 +0000 : com2 I would say either committed to multilingual / localization or not. Calling it a feature seems to say "translation has a low priority". When that is the case, why all the trouble to localize? Jus drop localization and make life easier! All depends on the chosen concept of Drupal. If you choose Drupal to be multilingual / localized, then this is definitely is a bug. You cannot expect a non-english speaker to understand these special users. What happens when I just translate them in table /role/? Would it break Drupal? ------------------------------------------------------------------------ Wed, 04 May 2005 13:48:34 +0000 : schalker As I can tell, standard roles still can not be translated. Is this correct? Is there a work-around or some plan to implement this? I'd agree with the former poster com2 that this is more a bug than a feature request: I love the locale module, but I cannot see a reason why almost everything is translateable, but not the standard roles. If there is no reason for this behaviour /from a user-point-of-view/, then this is a bug, I'd say. ------------------------------------------------------------------------ Wed, 04 May 2005 14:05:33 +0000 : schalker Sorry, it's me again. I searched on and on to find a similar bug report, but I haven't found one. So as two people think this is a bug, I change the category of this issue back to "bug report".

1 0

[drupal-devel] [bug] RSS feed should use created date, not updated date
by killes 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/23750 Post a follow up: http://drupal.org/project/comments/add/23750 Project: Drupal Version: 4.6.0 Component: node.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: jerakeen Updated by: killes(a)www.drop.org -Status: active +Status: patch (code needs review) It's a patch. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Fri, 27 May 2005 16:53:50 +0000 : jerakeen Attachment: http://drupal.org/files/issues/created_date_not_updated_date.patch (592 bytes) The pubDate in the RSS feed should use the node creation timestamp to set the pubDate, not the node updated timestamp. This might be the same as http://drupal.org/node/20368, but that issue isn't in the node component, and I don't want to jump to conclusions. Trivial patch attached.

1 0

[drupal-devel] [bug] User.module links for blocked/non-existant accounts + menu 403/404 issue
by killes 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/14591 Post a follow up: http://drupal.org/project/comments/add/14591 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Steven Reported by: Steven Updated by: killes(a)www.drop.org -Status: active +Status: patch (code needs work) Changing status. I think the solution proposed makes sense. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Sun, 19 Dec 2004 23:15:49 +0000 : Steven Attachment: http://drupal.org/files/issues/user.blocked.patch (1.53 KB) There is a bug in user.module that admins get a "page not found" message for blocked accounts, even though they should be allowed to see them. I also think that blocked accounts should return a 403 for non-admins, not a 404. The attached patch fixes these problems. However, in experimenting with possible solutions I uncovered a weird menu issue. Tracker.module and statistics.module register "user/N/track". Contact registers a "user/N/contact" tab. These tabs are dynamic and appear for all paths with the format "user/N". If "user/N" decides to throw a 403 or 404 then the other tabs will still appear, as the menu system hasn't changed its location. You can see this in action at http://drupal.org/user/15396. This account is blocked. The cause is that the 403/404 checks for the tabs are each done in their own menu handler (invoking drupal_not_found() or drupal_access_denied()), and not in hook_menu. However moving the checks to hook_menu would mean duplicating this check (and the queries) across several modules, and we'd have to avoid the menu falling down from "user/X/Y" to "user" (which has its own tabs, if you're an anonymous user) if the menu handler is not registered. Note that if custom 403/404 pages are used, then no tabs will show up as menu_set_location() is called to move to the custom 403/404 page, which is located somewhere else on the menu tree. This is further amplified by the fact that the access rules for the different tabs are inconsistent (some don't respect admin overrides, some don't respect blocked status). We can fix this problem by calling menu_set_location() with some dummy location in drupal_not_found() and drupal_access_denied(), thus ensuring we're somewhere else in the menu tree. In that case, the tabs won't appear for any 403/404. If we make the access rules across the different tabs consistent, then what you see in the UI (no tabs) will match the access rules. What do you guys think? ------------------------------------------------------------------------ Mon, 20 Dec 2004 08:59:45 +0000 : killes(a)www.drop.org I'd like this to get fixed, but I think your patch is incomplete for technical reasons: ***** CVS exited normally with code 1 ***** ------------------------------------------------------------------------ Mon, 20 Dec 2004 14:30:04 +0000 : Steven The patch only fixes the 403/404 and does not address the tabs issue yet, as I was not sure what the best approach was. The "*** CVS" thing is normal. ------------------------------------------------------------------------ Sun, 13 Mar 2005 18:09:42 +0000 : killes(a)www.drop.org Doesn't apply anymore.

1 0

[drupal-devel] [bug] request_uri() broken if in subdir
by Robin Monks 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/10917 Post a follow up: http://drupal.org/project/comments/add/10917 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal -Assigned to: Robin Monks +Assigned to: Anonymous Reported by: killes(a)www.drop.org Updated by: Robin Monks Status: patch (code needs work) I'm removing myself from this bug, as I can't adequately understand it to make a quality patch. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ Fri, 17 Sep 2004 19:05:57 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/requi.patch (678 bytes) We asumme the $_SERVER[REQUEST_URI] would contain only the url from the point of Drupal's directory on. This is true only iff you run from the root of your site. If you are in a subdir, the function request_uri() will be unusable if we want to construct a valid url from $base_url and request_uri() (what we do in a number of places). The attached patch fixes this by using basename(). ------------------------------------------------------------------------ Fri, 17 Sep 2004 19:45:26 +0000 : killes(a)www.drop.org The patch doesn't suffice if your access only the base_url of your Drupal site in a subdir. Putting on hold. ------------------------------------------------------------------------ Fri, 17 Sep 2004 21:39:36 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/request.patch (761 bytes) ok, I think I got it. The distinction between apache and non-apache servers went away to keep it simple. ------------------------------------------------------------------------ Sat, 18 Sep 2004 00:19:57 +0000 : killes(a)www.drop.org Apparently the action in forms depends on the bug that I found, ie it needs request_uri to contain the full url (from documentroot on). ------------------------------------------------------------------------ Sat, 18 Sep 2004 00:53:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/request_0.patch (2.46 KB) This new patch fixes the problem with forms. request_uri() also does not start with a leading slash anymore. Ask Steven for why this is good. http://acko.net/dumpx/relative.html is apparently a clue I don't get. ------------------------------------------------------------------------ Sat, 18 Sep 2004 01:04:41 +0000 : Steven Explanation: People are going to stick relative_uri() into links. These links are resolved relative to the $base_url which is put into the <head> tag. However, if a link URI starts with a / then it is relative to the root of the site, even if the base tag uses a path with a subdirectory. Example: <html> <base href="http://www.test.com/subdir1/"> <body> test [1] test [2] </body> </html> The first link resolves to http://www.test.com/subdir2, the second resolves to http://www.test.com/subdir1/subdir2. Thus, it is more meaningful to not have a leading slash, as this will avoid possible bugs with subdirs (which often do not get caught because the tester uses a site without a subdir) and hints at the callee that this is a relative URI too. [1] http://drupal.org//subdir2 [2] http://drupal.org/subdir2 ------------------------------------------------------------------------ Sun, 13 Mar 2005 18:38:21 +0000 : Chris Johnson Patch no longer applies completely to bootstrap.inc: Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: includes/bootstrap.inc |=================================================================== |RCS file: /cvs/drupal/drupal/includes/bootstrap.inc,v |retrieving revision 1.28 |diff -u -F^f -r1.28 bootstrap.inc |--- includes/bootstrap.inc 16 Sep 2004 07:17:54 -0000 1.28 |+++ includes/bootstrap.inc 18 Sep 2004 00:52:12 -0000 -------------------------- Patching file bootstrap.inc using Plan A... Hunk #1 succeeded at 273 (offset 83 lines). Hunk #2 succeeded at 292 (offset 83 lines). Hunk #3 failed at 446. 1 out of 3 hunks failed--saving rejects to bootstrap.inc.rej Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: includes/common.inc |=================================================================== |RCS file: /cvs/drupal/drupal/includes/common.inc,v |retrieving revision 1.390 |diff -u -F^f -r1.390 common.inc |--- includes/common.inc 16 Sep 2004 16:12:21 -0000 1.390 |+++ includes/common.inc 18 Sep 2004 00:52:12 -0000 -------------------------- Patching file common.inc using Plan A... Hunk #1 succeeded at 988 (offset -90 lines). done ------------------------------------------------------------------------ Tue, 14 Jun 2005 11:21:49 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/request_uri.broken.if.in.subdir.patch (2.6 KB) Here is that patch re-rolled for latest CVS HEAD. Since killes hasn't been active in this bug lately, I hope he won't mind my reassigning this to myself. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 11:22:40 +0000 : Robin Monks I've also tested this to work on CVS HEAD. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 13:33:52 +0000 : moshe weitzman did you test this on non apache servers? the comment that you removed is crystal clear about a need for 2 different cases. it is OK to 'make it simpler' as long as you don't remove compatibility with other web servers. I am primarily concerned with IIS. ------------------------------------------------------------------------ Tue, 14 Jun 2005 14:42:44 +0000 : Robin Monks I took great care to ensure I didn't remove any support needed for other servers. I also tested this patch to work on Xitami on Win32. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 17:09:29 +0000 : Robin Monks I spent some more time testing, and this broke some (perhaps all) configuration forms. It appears the / is needed after all. Perhaps killes will have better luck understanding this, the / is needed IMHO. Robin ------------------------------------------------------------------------ Fri, 01 Jul 2005 13:20:27 +0000 : killes(a)www.drop.org Another manifestation of this bug: http://drupal.org/node/26163 ------------------------------------------------------------------------ Tue, 09 Aug 2005 17:26:44 +0000 : killes(a)www.drop.org Another occurrence: http://drupal.org/node/20555

1 0

[drupal-devel] [bug] request_uri() broken if in subdir
by killes 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/10917 Post a follow up: http://drupal.org/project/comments/add/10917 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org -Status: patch (code needs review) +Status: patch (code needs work) Another occurrence: http://drupal.org/node/20555 killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Fri, 17 Sep 2004 19:05:57 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/requi.patch (678 bytes) We asumme the $_SERVER[REQUEST_URI] would contain only the url from the point of Drupal's directory on. This is true only iff you run from the root of your site. If you are in a subdir, the function request_uri() will be unusable if we want to construct a valid url from $base_url and request_uri() (what we do in a number of places). The attached patch fixes this by using basename(). ------------------------------------------------------------------------ Fri, 17 Sep 2004 19:45:26 +0000 : killes(a)www.drop.org The patch doesn't suffice if your access only the base_url of your Drupal site in a subdir. Putting on hold. ------------------------------------------------------------------------ Fri, 17 Sep 2004 21:39:36 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/request.patch (761 bytes) ok, I think I got it. The distinction between apache and non-apache servers went away to keep it simple. ------------------------------------------------------------------------ Sat, 18 Sep 2004 00:19:57 +0000 : killes(a)www.drop.org Apparently the action in forms depends on the bug that I found, ie it needs request_uri to contain the full url (from documentroot on). ------------------------------------------------------------------------ Sat, 18 Sep 2004 00:53:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/request_0.patch (2.46 KB) This new patch fixes the problem with forms. request_uri() also does not start with a leading slash anymore. Ask Steven for why this is good. http://acko.net/dumpx/relative.html is apparently a clue I don't get. ------------------------------------------------------------------------ Sat, 18 Sep 2004 01:04:41 +0000 : Steven Explanation: People are going to stick relative_uri() into links. These links are resolved relative to the $base_url which is put into the <head> tag. However, if a link URI starts with a / then it is relative to the root of the site, even if the base tag uses a path with a subdirectory. Example: <html> <base href="http://www.test.com/subdir1/"> <body> test [1] test [2] </body> </html> The first link resolves to http://www.test.com/subdir2, the second resolves to http://www.test.com/subdir1/subdir2. Thus, it is more meaningful to not have a leading slash, as this will avoid possible bugs with subdirs (which often do not get caught because the tester uses a site without a subdir) and hints at the callee that this is a relative URI too. [1] http://drupal.org//subdir2 [2] http://drupal.org/subdir2 ------------------------------------------------------------------------ Sun, 13 Mar 2005 18:38:21 +0000 : Chris Johnson Patch no longer applies completely to bootstrap.inc: Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: includes/bootstrap.inc |=================================================================== |RCS file: /cvs/drupal/drupal/includes/bootstrap.inc,v |retrieving revision 1.28 |diff -u -F^f -r1.28 bootstrap.inc |--- includes/bootstrap.inc 16 Sep 2004 07:17:54 -0000 1.28 |+++ includes/bootstrap.inc 18 Sep 2004 00:52:12 -0000 -------------------------- Patching file bootstrap.inc using Plan A... Hunk #1 succeeded at 273 (offset 83 lines). Hunk #2 succeeded at 292 (offset 83 lines). Hunk #3 failed at 446. 1 out of 3 hunks failed--saving rejects to bootstrap.inc.rej Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: includes/common.inc |=================================================================== |RCS file: /cvs/drupal/drupal/includes/common.inc,v |retrieving revision 1.390 |diff -u -F^f -r1.390 common.inc |--- includes/common.inc 16 Sep 2004 16:12:21 -0000 1.390 |+++ includes/common.inc 18 Sep 2004 00:52:12 -0000 -------------------------- Patching file common.inc using Plan A... Hunk #1 succeeded at 988 (offset -90 lines). done ------------------------------------------------------------------------ Tue, 14 Jun 2005 11:21:49 +0000 : Robin Monks Attachment: http://drupal.org/files/issues/request_uri.broken.if.in.subdir.patch (2.6 KB) Here is that patch re-rolled for latest CVS HEAD. Since killes hasn't been active in this bug lately, I hope he won't mind my reassigning this to myself. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 11:22:40 +0000 : Robin Monks I've also tested this to work on CVS HEAD. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 13:33:52 +0000 : moshe weitzman did you test this on non apache servers? the comment that you removed is crystal clear about a need for 2 different cases. it is OK to 'make it simpler' as long as you don't remove compatibility with other web servers. I am primarily concerned with IIS. ------------------------------------------------------------------------ Tue, 14 Jun 2005 14:42:44 +0000 : Robin Monks I took great care to ensure I didn't remove any support needed for other servers. I also tested this patch to work on Xitami on Win32. Robin ------------------------------------------------------------------------ Tue, 14 Jun 2005 17:09:29 +0000 : Robin Monks I spent some more time testing, and this broke some (perhaps all) configuration forms. It appears the / is needed after all. Perhaps killes will have better luck understanding this, the / is needed IMHO. Robin ------------------------------------------------------------------------ Fri, 01 Jul 2005 13:20:27 +0000 : killes(a)www.drop.org Another manifestation of this bug: http://drupal.org/node/26163

1 0

[drupal-devel] [bug] Small patch to sort "new users" correctly
by killes 09 Aug '05

09 Aug '05

Issue status update for http://drupal.org/node/18778 Post a follow up: http://drupal.org/project/comments/add/18778 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Olen Updated by: killes(a)www.drop.org -Status: active +Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/usersort.patch (767 bytes) Right. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Sat, 12 Mar 2005 10:53:00 +0000 : Olen After importing and merging several drupal installastions, i came across this "bug". Nothing important, but I think it's better to sort the users after "created" than "uid" when getting the latest aditions. --- user.module.orig 2005-03-12 11:48:54.485881868 +0100 +++ user.module 2005-03-12 11:49:06.146192209 +0100 @@ -526,7 +526,7 @@ case 2: if (user_access('access content')) { - $result = db_query_range('SELECT uid, name FROM {users} WHERE status != 0 ORDER BY uid DESC', 0, 5); + $result = db_query_range('SELECT uid, name FROM {users} WHERE status != 0 ORDER BY created DESC', 0, 5); while ($account = db_fetch_object($result)) { $items[] = format_name($account); }

1 0