- Development - lists.drupal.org

[drupal-devel] [feature] disable export
by moshe weitzman 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/27999 Post a follow up: http://drupal.org/project/comments/add/27999 Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Tobias Maier Updated by: moshe weitzman -Status: patch (code needs review) +Status: patch (code needs work) in addiiton to removing links, you have to block access tp these features. this is done with the 'access' element of the relevant items in book_menu() moshe weitzman Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 23:32:14 +0000 : Tobias Maier Hello, it would be good to be able to deactivate * printer-friendly version * export DocBook XML and export OPML I dont want that people are able to export my content, they should just be able to see it and to print it with my special print css-style-sheed (and just the specific page) ------------------------------------------------------------------------ Wed, 03 Aug 2005 15:19:26 +0000 : Tobias Maier Attachment: http://drupal.org/files/issues/book.module_5.patch (1.85 KB) here comes my patch of this. it introduces two more permissions: 'export books' and 'see printer-friendly version' why so I want to hide the printer-friendly version? * Not every User wants to print the Subpages * I want to provide a special print style sheed (@media print or how it was called in css) so I dont need a completely new site, which will be generated by drupal The User should see my logo on every page - this is not possible with "printer-friendly version" * It does not fit for a corporate site (my opinion) which I want to make with drupal read http://support.bryght.com/adminguide/site-types/corporate

1 0

[drupal-devel] [task] Starting a forum topic should display signature just like in comments
by moshe weitzman 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/10938 Post a follow up: http://drupal.org/project/comments/add/10938 Project: Drupal Version: cvs Component: forum.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: moshe weitzman -Status: patch (code needs review) +Status: patch (code needs work) should implement for all nodes, not just forum ... ideally we move sig code out of comment and into common.inc or user.module moshe weitzman Previous comments: ------------------------------------------------------------------------ Sun, 19 Sep 2004 12:11:23 +0000 : robertDouglass I don't see any reason why the user signature that is provided when writing comments shouldn't be there when forum topics are created as well. ------------------------------------------------------------------------ Mon, 15 Aug 2005 16:24:17 +0000 : webchick Attachment: http://drupal.org/files/issues/forum_signature.patch (947 bytes) Here is a very simple patch that just does this the same way as it's done in comment.module.

1 0

[drupal-devel] [bug] Handle full URLs in url() and l()
by jsloan 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/10888 Post a follow up: http://drupal.org/project/comments/add/10888 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Bèr Kessels Reported by: jhriggs Updated by: jsloan Status: patch (code needs work) ... the patch I am referring to is above at # 23 jsloan Previous comments: ------------------------------------------------------------------------ Thu, 16 Sep 2004 15:01:24 +0000 : jhriggs Attachment: http://drupal.org/files/issues/full_url.patch (1.97 KB) This little patch allows full URLs to be used for the url() and l() functions, giving modules a single interface for creating links. Currently, modules must use l() for drupal links and raw HTML for external links. This patch changes url() to handle -- basically pass through -- full URLs while still doing the proper handling of internal paths and works with clean URLs enabled or disabled. It also shortens (and hopefully simplifies) the url() code a bit. Some examples of calls that now work: <?php l('a link', 'some/path'); // same as before l('an external link', 'http://drupal.org/'); l('full with query', 'http://google.com/search?q=define:foo'); l('full with separate query', 'http://google.com/search', array(), 'q=define:foo'); // same link as above l('full with fragment', 'http://drupal.org/node/5942#comment-12374'); l('full with separate fragment', 'http://drupal.org/node/5942', array(), NULL, 'comment-12374'); // same link as above ?> ------------------------------------------------------------------------ Thu, 30 Sep 2004 11:09:30 +0000 : moshe weitzman +1. less HTML in code is a good thing for readability, if nothing else. ------------------------------------------------------------------------ Mon, 24 Jan 2005 20:06:26 +0000 : mathias Attachment: http://drupal.org/files/issues/external_urls.patch (838 bytes) Revisiting this issue one year later shows we're almost there. The only thing that's left is the ability to handle external urls when clean_urls is disabled. Once implemented, menu.module can happily accept external urls and not just Drupal paths, making the menu system much more attractive to end users. I'll update the menu.module docs if this patch (or an alternative solution) is accepted. ------------------------------------------------------------------------ Sun, 13 Mar 2005 22:36:34 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/external_urls_0.patch (909 bytes) Updated from Drupal root directory. ------------------------------------------------------------------------ Mon, 14 Mar 2005 00:05:17 +0000 : gordon +1 This is a good thing for people who can't using clean urls. ------------------------------------------------------------------------ Sat, 02 Apr 2005 06:23:10 +0000 : TDobes This patch seems like a good idea, but does not apply anymore. Also, shouldn't the comments for l() and url() be updated to indicate that the functions will work with both internal and external URL's? ------------------------------------------------------------------------ Sat, 02 Apr 2005 07:25:13 +0000 : asimmonds Attachment: http://drupal.org/files/issues/external_urls_1.patch (897 bytes) The &'s had been changed to & amp; , here's a valid patch ------------------------------------------------------------------------ Thu, 12 May 2005 14:57:07 +0000 : adrian This is a new patch against cvs that allows url() to create external links, with the side-effect that menu can now be used to make external links. Also closes http://drupal.org/node/10177 This will be even more important with the primary links menu integration. (This patch already cleans up a special case in phptemplate for that). Killes benchmarked this, and it turns out the difference is negligible. Requests per second: 1.33 [#/sec] (mean) = with patch Requests per second: 1.31 [#/sec] (mean) = without patch. ------------------------------------------------------------------------ Thu, 12 May 2005 14:59:57 +0000 : killes(a)www.drop.org I had benchmarked /node as anon user without cache. ------------------------------------------------------------------------ Wed, 25 May 2005 04:50:08 +0000 : Steven +1 on the idea of this patch. It allows people to use l() for external links. However, I don't like the implementation. Running that complicated regexp for every url() call is simply wasteful. 95% or more of url() calls have a fixed string as parameter and point to Drupal's own paths. I suggest we create an url_external() wrapper around url() which allows external URLs. Any path which we want to allow external URLs in can use url_external() instead. The places where we use it will not be many anyway. ------------------------------------------------------------------------ Wed, 25 May 2005 05:05:55 +0000 : gordon splitting into url() url_external is not a good idea. The main reason for this patch that I see is for putting external urls into the navigation menu using the menu.module, and if all goes to plan, the primary and secondary menus. ------------------------------------------------------------------------ Wed, 25 May 2005 06:00:01 +0000 : Steven gordon: I still think this is a bit excessive. Perhaps instead of valid_url() we could use a simpler regexp !^[a-z]://!i or even strpos('://'). ------------------------------------------------------------------------ Wed, 25 May 2005 06:15:36 +0000 : gordon I am not says that it is or is not excessive. I was just saying that this is where it is going to be used, so creating url() and url_external() is not really an option ------------------------------------------------------------------------ Wed, 25 May 2005 06:57:53 +0000 : Bèr Kessels we /do/ need an external_url() thing. The links bundle (handling weblinks and the likes) willprovide this /and/ need this. People will get all sorts of tools to do /stuff/ with external links, like add permissions, track clicks etc. depending on the modules they have and the settings. All outgoing weblinks, should somehow respect those. Patches for core are coming, though. On top of that, a external_l() finction is simple. And if people want them in primary links they can do the regexping /before/ the function, not /in/ the function. The calling code can decide whether to call l() or external_l(), instead of putting logic in l() that is unneded 95% of the times. But i hereby promise to introduce such a function tbefore the end of this week :) ------------------------------------------------------------------------ Wed, 25 May 2005 07:17:47 +0000 : gordon The only thing I do not see in this plan is how with this work with the menu module and the navigation menu. ------------------------------------------------------------------------ Wed, 25 May 2005 10:04:12 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links.patch (1.75 KB) What happened to the patch i uploaded ? It doesn't use a complex regexp, but uses strpos($url, '://') < 5. ------------------------------------------------------------------------ Thu, 26 May 2005 06:19:32 +0000 : Bèr Kessels Allthough this works, is clean AND does what it should do, I still am in favour of a better nitegrated solution. So please do not yet commit this, untill I upload my patch. We can then discuss what solution to take. It will not differ a big lot though. ------------------------------------------------------------------------ Thu, 26 May 2005 08:39:32 +0000 : adrian is this the patch where you want to introduce an el() to allow for counting of external links ? That's a different patch altogether. This is primarily about stopping url() from not handling external links when there is no good reason for it to do so. Your el() has a very specific use in the weblinks api, and should probably be included with it. ------------------------------------------------------------------------ Thu, 26 May 2005 16:18:31 +0000 : Bèr Kessels Yes, that el() But let me explain why i am anxious for my solution: el(), by default will not do much, in fact it will just be an URL()-clone but one that allows outgoing links. But it has a hook-caller. Again: by default it does nothing much, for there will not be any hooks around in core. It will allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link. Now, the whole issue, IMO is, that we should *not* provide two ways for handling external URLS, but rather a single one, that is weblink-safe. If we would introduce URL() with outgoing links-capabilities, we can no longer track any outgoing URLs trough them. We will then have two ways for handling outgoing URLs, resulting in inconsistencies, and confused users. (hey, my anonymous users can follow link foo, while i have "follow outgig links" permission disabled for them). (BTW weblink will soon be renamed into links, just for reference) ------------------------------------------------------------------------ Thu, 30 Jun 2005 11:06:11 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links_0.patch (824 bytes) I've updated the patch. ------------------------------------------------------------------------ Mon, 18 Jul 2005 14:12:36 +0000 : adrian spoke to dries about this patch, and apart from the incomplete patch I uploaded, there are about a dozen instances where in-line html is used were l() would suffice. I will be cleaning them up and submitting a new patch soon. ------------------------------------------------------------------------ Fri, 05 Aug 2005 18:44:03 +0000 : moshe weitzman hopefully adrian or someone else will chase down the remaining urls which should use this function. ------------------------------------------------------------------------ Fri, 05 Aug 2005 19:25:08 +0000 : jsloan I've come to this party late but would like to see a patch soon... I've used the patch from Adrian (#15) but this did not work correctly in my setup (IIS 5.0 / no clean urls) I applied it against 4.6.2 and cvs. Where is the latest patch that I can test ... I'll throw some time at this because I need it for a module used within our Intranet. ------------------------------------------------------------------------ Mon, 08 Aug 2005 20:06:03 +0000 : jsloan Attachment: http://drupal.org/files/issues/external_urls_2.patch (3.67 KB) I'm not sure what the final plans are for this in the CVS so I have developed a patch for use on 4.6.2 I decided to add a parameter named $external that is passed to url() function. The default is FALSE so this should not break any existing calls to either l() or url(). I have also refactored url() so that it is much easier to understand and extend --- I'm thinking of Bèr Kessels intention to "allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link" " Since the parameter is passed to activate the external url there is no need for regex or strpos, this should be handled by the calling function and then set the $external parameter to TRUE For example I have patched the theme_menu_item_link() function in menu.inc to accept external urls: <?php function theme_menu_item_link($item, $link_item) { // this next line was added so that a menu item could point to the default home $link_item['path'] = preg_replace('/^\/$/','',$link_item['path']); // this next line was added so that a menu item could point to an external url $external = (preg_match('<^[a-z]+://>', $link_item['path'])) ? TRUE : FALSE ; return l($item['title'], $link_item['path'], array_key_exists('description', $item) ? array('title' => $items['description']) : array(),NULL,NULL,FALSE,FALSE,$external); } ?> I hope that this patch will spur some solutions - I'm using this now in production but have not tested extensively across every scenario. thanks jim sloan ------------------------------------------------------------------------ Fri, 19 Aug 2005 12:10:50 +0000 : jsloan ... are there any thoughts on this? What is the status of the final patch? ------------------------------------------------------------------------ Sun, 28 Aug 2005 15:48:08 +0000 : adrian The problem with this patch is that we still need to fix all the inline occurrences of the <a href tag. I don't think we should confuse the issue and implement a hook at this point, as url() breaking with external links is a bug, pure and simple. ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:24:16 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.inc.patch.txt (1.16 KB) Adrian, your second patch for theme.inc looks wrong to me. Don't you still need the line $settings[$type .'_links'][] = l($text, $link, $attributes); ? I also got rid of unset($attributes); Where was that supposed to come from that it needed unsetting? ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:48:02 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.common.inc.patch.txt (1.83 KB) Here is a unified patch that I updated to fix a bug in Adrian's common.inc code (everything was being handled as absolute), plus the update to theme.inc that I questioned in the previous post. In my initial testing, this works perfectly. I can put absolute URLs in for the paths in menu items and it handles them fine, and changes no other behavior. I searched core for all uses of url() and http:// and found no more violating code such as that in theme.inc. Net change in core with this patch: 0 lines of code. Please review quickly, I'm writing in the upcoming book about this functionality as if it is in. =) ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:49:43 +0000 : robertDouglass Correction: net change to core -1 line of code. ------------------------------------------------------------------------ Thu, 08 Sep 2005 18:58:01 +0000 : adrian What needs to be removed from core are not uses of url() and l(), but all occurences of the anchor tag. The anchor tag was mostly used because l() didn't do external links. Also, in the case of phptemplate , that is the line that creates the array that is passed to the template. ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:58:13 +0000 : robertDouglass Good - that needs to be removed too, but is there anything that doesn't work right if we don't remove them? Lots of the $output = '<a href....' instances in the code use url() for href and these are not broken. They can't be seen as a reason not to commit this particular patch. If anyone point out things that break with the patch I made, I'll fix them. I can't go through and clean up anchor tags on idealistic grounds at the moment (as much as I would otherwise like to). ------------------------------------------------------------------------ Sat, 10 Sep 2005 20:11:08 +0000 : jsloan I think that using strpos($path, '://') to trigger the external url is a bad idea. My patch implements a new parameter to url() and l() that will let the calling function decided if it is an external url. And it is backward compatible with all existing calls. I also rewrote the logic of builing the url to make it simple. Adrian, I do not agree that it was a bug; url() never intended to handle external url´s and also, the clean up of the inline anchor tags are not dependent on this feature. Can this be considered for the solution to this feature?

1 0

[drupal-devel] [bug] Handle full URLs in url() and l()
by jsloan 10 Sep '05

10 Sep '05

Issue status update for http://drupal.org/node/10888 Post a follow up: http://drupal.org/project/comments/add/10888 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Bèr Kessels Reported by: jhriggs Updated by: jsloan Status: patch (code needs work) I think that using strpos($path, '://') to trigger the external url is a bad idea. My patch implements a new parameter to url() and l() that will let the calling function decided if it is an external url. And it is backward compatible with all existing calls. I also rewrote the logic of builing the url to make it simple. Adrian, I do not agree that it was a bug; url() never intended to handle external url´s and also, the clean up of the inline anchor tags are not dependent on this feature. Can this be considered for the solution to this feature? jsloan Previous comments: ------------------------------------------------------------------------ Thu, 16 Sep 2004 15:01:24 +0000 : jhriggs Attachment: http://drupal.org/files/issues/full_url.patch (1.97 KB) This little patch allows full URLs to be used for the url() and l() functions, giving modules a single interface for creating links. Currently, modules must use l() for drupal links and raw HTML for external links. This patch changes url() to handle -- basically pass through -- full URLs while still doing the proper handling of internal paths and works with clean URLs enabled or disabled. It also shortens (and hopefully simplifies) the url() code a bit. Some examples of calls that now work: <?php l('a link', 'some/path'); // same as before l('an external link', 'http://drupal.org/'); l('full with query', 'http://google.com/search?q=define:foo'); l('full with separate query', 'http://google.com/search', array(), 'q=define:foo'); // same link as above l('full with fragment', 'http://drupal.org/node/5942#comment-12374'); l('full with separate fragment', 'http://drupal.org/node/5942', array(), NULL, 'comment-12374'); // same link as above ?> ------------------------------------------------------------------------ Thu, 30 Sep 2004 11:09:30 +0000 : moshe weitzman +1. less HTML in code is a good thing for readability, if nothing else. ------------------------------------------------------------------------ Mon, 24 Jan 2005 20:06:26 +0000 : mathias Attachment: http://drupal.org/files/issues/external_urls.patch (838 bytes) Revisiting this issue one year later shows we're almost there. The only thing that's left is the ability to handle external urls when clean_urls is disabled. Once implemented, menu.module can happily accept external urls and not just Drupal paths, making the menu system much more attractive to end users. I'll update the menu.module docs if this patch (or an alternative solution) is accepted. ------------------------------------------------------------------------ Sun, 13 Mar 2005 22:36:34 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/external_urls_0.patch (909 bytes) Updated from Drupal root directory. ------------------------------------------------------------------------ Mon, 14 Mar 2005 00:05:17 +0000 : gordon +1 This is a good thing for people who can't using clean urls. ------------------------------------------------------------------------ Sat, 02 Apr 2005 06:23:10 +0000 : TDobes This patch seems like a good idea, but does not apply anymore. Also, shouldn't the comments for l() and url() be updated to indicate that the functions will work with both internal and external URL's? ------------------------------------------------------------------------ Sat, 02 Apr 2005 07:25:13 +0000 : asimmonds Attachment: http://drupal.org/files/issues/external_urls_1.patch (897 bytes) The &'s had been changed to & amp; , here's a valid patch ------------------------------------------------------------------------ Thu, 12 May 2005 14:57:07 +0000 : adrian This is a new patch against cvs that allows url() to create external links, with the side-effect that menu can now be used to make external links. Also closes http://drupal.org/node/10177 This will be even more important with the primary links menu integration. (This patch already cleans up a special case in phptemplate for that). Killes benchmarked this, and it turns out the difference is negligible. Requests per second: 1.33 [#/sec] (mean) = with patch Requests per second: 1.31 [#/sec] (mean) = without patch. ------------------------------------------------------------------------ Thu, 12 May 2005 14:59:57 +0000 : killes(a)www.drop.org I had benchmarked /node as anon user without cache. ------------------------------------------------------------------------ Wed, 25 May 2005 04:50:08 +0000 : Steven +1 on the idea of this patch. It allows people to use l() for external links. However, I don't like the implementation. Running that complicated regexp for every url() call is simply wasteful. 95% or more of url() calls have a fixed string as parameter and point to Drupal's own paths. I suggest we create an url_external() wrapper around url() which allows external URLs. Any path which we want to allow external URLs in can use url_external() instead. The places where we use it will not be many anyway. ------------------------------------------------------------------------ Wed, 25 May 2005 05:05:55 +0000 : gordon splitting into url() url_external is not a good idea. The main reason for this patch that I see is for putting external urls into the navigation menu using the menu.module, and if all goes to plan, the primary and secondary menus. ------------------------------------------------------------------------ Wed, 25 May 2005 06:00:01 +0000 : Steven gordon: I still think this is a bit excessive. Perhaps instead of valid_url() we could use a simpler regexp !^[a-z]://!i or even strpos('://'). ------------------------------------------------------------------------ Wed, 25 May 2005 06:15:36 +0000 : gordon I am not says that it is or is not excessive. I was just saying that this is where it is going to be used, so creating url() and url_external() is not really an option ------------------------------------------------------------------------ Wed, 25 May 2005 06:57:53 +0000 : Bèr Kessels we /do/ need an external_url() thing. The links bundle (handling weblinks and the likes) willprovide this /and/ need this. People will get all sorts of tools to do /stuff/ with external links, like add permissions, track clicks etc. depending on the modules they have and the settings. All outgoing weblinks, should somehow respect those. Patches for core are coming, though. On top of that, a external_l() finction is simple. And if people want them in primary links they can do the regexping /before/ the function, not /in/ the function. The calling code can decide whether to call l() or external_l(), instead of putting logic in l() that is unneded 95% of the times. But i hereby promise to introduce such a function tbefore the end of this week :) ------------------------------------------------------------------------ Wed, 25 May 2005 07:17:47 +0000 : gordon The only thing I do not see in this plan is how with this work with the menu module and the navigation menu. ------------------------------------------------------------------------ Wed, 25 May 2005 10:04:12 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links.patch (1.75 KB) What happened to the patch i uploaded ? It doesn't use a complex regexp, but uses strpos($url, '://') < 5. ------------------------------------------------------------------------ Thu, 26 May 2005 06:19:32 +0000 : Bèr Kessels Allthough this works, is clean AND does what it should do, I still am in favour of a better nitegrated solution. So please do not yet commit this, untill I upload my patch. We can then discuss what solution to take. It will not differ a big lot though. ------------------------------------------------------------------------ Thu, 26 May 2005 08:39:32 +0000 : adrian is this the patch where you want to introduce an el() to allow for counting of external links ? That's a different patch altogether. This is primarily about stopping url() from not handling external links when there is no good reason for it to do so. Your el() has a very specific use in the weblinks api, and should probably be included with it. ------------------------------------------------------------------------ Thu, 26 May 2005 16:18:31 +0000 : Bèr Kessels Yes, that el() But let me explain why i am anxious for my solution: el(), by default will not do much, in fact it will just be an URL()-clone but one that allows outgoing links. But it has a hook-caller. Again: by default it does nothing much, for there will not be any hooks around in core. It will allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link. Now, the whole issue, IMO is, that we should *not* provide two ways for handling external URLS, but rather a single one, that is weblink-safe. If we would introduce URL() with outgoing links-capabilities, we can no longer track any outgoing URLs trough them. We will then have two ways for handling outgoing URLs, resulting in inconsistencies, and confused users. (hey, my anonymous users can follow link foo, while i have "follow outgig links" permission disabled for them). (BTW weblink will soon be renamed into links, just for reference) ------------------------------------------------------------------------ Thu, 30 Jun 2005 11:06:11 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links_0.patch (824 bytes) I've updated the patch. ------------------------------------------------------------------------ Mon, 18 Jul 2005 14:12:36 +0000 : adrian spoke to dries about this patch, and apart from the incomplete patch I uploaded, there are about a dozen instances where in-line html is used were l() would suffice. I will be cleaning them up and submitting a new patch soon. ------------------------------------------------------------------------ Fri, 05 Aug 2005 18:44:03 +0000 : moshe weitzman hopefully adrian or someone else will chase down the remaining urls which should use this function. ------------------------------------------------------------------------ Fri, 05 Aug 2005 19:25:08 +0000 : jsloan I've come to this party late but would like to see a patch soon... I've used the patch from Adrian (#15) but this did not work correctly in my setup (IIS 5.0 / no clean urls) I applied it against 4.6.2 and cvs. Where is the latest patch that I can test ... I'll throw some time at this because I need it for a module used within our Intranet. ------------------------------------------------------------------------ Mon, 08 Aug 2005 20:06:03 +0000 : jsloan Attachment: http://drupal.org/files/issues/external_urls_2.patch (3.67 KB) I'm not sure what the final plans are for this in the CVS so I have developed a patch for use on 4.6.2 I decided to add a parameter named $external that is passed to url() function. The default is FALSE so this should not break any existing calls to either l() or url(). I have also refactored url() so that it is much easier to understand and extend --- I'm thinking of Bèr Kessels intention to "allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link" " Since the parameter is passed to activate the external url there is no need for regex or strpos, this should be handled by the calling function and then set the $external parameter to TRUE For example I have patched the theme_menu_item_link() function in menu.inc to accept external urls: <?php function theme_menu_item_link($item, $link_item) { // this next line was added so that a menu item could point to the default home $link_item['path'] = preg_replace('/^\/$/','',$link_item['path']); // this next line was added so that a menu item could point to an external url $external = (preg_match('<^[a-z]+://>', $link_item['path'])) ? TRUE : FALSE ; return l($item['title'], $link_item['path'], array_key_exists('description', $item) ? array('title' => $items['description']) : array(),NULL,NULL,FALSE,FALSE,$external); } ?> I hope that this patch will spur some solutions - I'm using this now in production but have not tested extensively across every scenario. thanks jim sloan ------------------------------------------------------------------------ Fri, 19 Aug 2005 12:10:50 +0000 : jsloan ... are there any thoughts on this? What is the status of the final patch? ------------------------------------------------------------------------ Sun, 28 Aug 2005 15:48:08 +0000 : adrian The problem with this patch is that we still need to fix all the inline occurrences of the <a href tag. I don't think we should confuse the issue and implement a hook at this point, as url() breaking with external links is a bug, pure and simple. ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:24:16 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.inc.patch.txt (1.16 KB) Adrian, your second patch for theme.inc looks wrong to me. Don't you still need the line $settings[$type .'_links'][] = l($text, $link, $attributes); ? I also got rid of unset($attributes); Where was that supposed to come from that it needed unsetting? ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:48:02 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.common.inc.patch.txt (1.83 KB) Here is a unified patch that I updated to fix a bug in Adrian's common.inc code (everything was being handled as absolute), plus the update to theme.inc that I questioned in the previous post. In my initial testing, this works perfectly. I can put absolute URLs in for the paths in menu items and it handles them fine, and changes no other behavior. I searched core for all uses of url() and http:// and found no more violating code such as that in theme.inc. Net change in core with this patch: 0 lines of code. Please review quickly, I'm writing in the upcoming book about this functionality as if it is in. =) ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:49:43 +0000 : robertDouglass Correction: net change to core -1 line of code. ------------------------------------------------------------------------ Thu, 08 Sep 2005 18:58:01 +0000 : adrian What needs to be removed from core are not uses of url() and l(), but all occurences of the anchor tag. The anchor tag was mostly used because l() didn't do external links. Also, in the case of phptemplate , that is the line that creates the array that is passed to the template. ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:58:13 +0000 : robertDouglass Good - that needs to be removed too, but is there anything that doesn't work right if we don't remove them? Lots of the $output = '<a href....' instances in the code use url() for href and these are not broken. They can't be seen as a reason not to commit this particular patch. If anyone point out things that break with the patch I made, I'll fix them. I can't go through and clean up anchor tags on idealistic grounds at the moment (as much as I would otherwise like to).

2 1

[drupal-devel] [feature] Extend access control options
by timcn 10 Sep '05

10 Sep '05

Issue status update for http://drupal.org/node/30711 Post a follow up: http://drupal.org/project/comments/add/30711 Project: Drupal Version: 4.6.3 Component: node.module Category: feature requests Priority: normal Assigned to: timcn Reported by: timcn Updated by: timcn Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_33.patch (8.24 KB) Unfortunately you can only give a role permission to "access content" and/or "administer nodes". But often, this does not suffice. You want editors to be able to browse the nodes in the "content" section and for example allow them to make a node sticky. But you don't want them to delete a node or change it's visibility. My patch offers the following permissions: * access content * administer nodes * change revision setting * delete nodes * make nodes sticky * moderate nodes * promote nodes It does also really check if the user is allowed to perform a certain action and deny it if that is not the case. It does also hide the specified checkboxes and options from the dropdown box. The patch is for 4.6.3 but should also work with HEAD. timcn

6 7

[drupal-devel] [feature] OPML Import for Aggregator
by arnabdotorg 10 Sep '05

10 Sep '05

Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: arnabdotorg Reported by: arnabdotorg Updated by: arnabdotorg Status: patch Attachment: http://drupal.org/files/issues/import_opml.patch (3.96 KB) The aggregator supports OPML export for all feeds, but not import. This patch enables import of OPML lists, allowing people to import lists of RSS feeds they've already assembled in another RSS app. Demo: http://arnab.drupaldevs.org/aggregator Patch: http://arnab.drupaldevs.org/tmp/import_opml.patch arnabdotorg -- View: http://drupal.org/node/16282 Edit: http://drupal.org/project/comments/add/16282

5 6

Re: [drupal-devel] Stop Anonymous users from even reaching a page and generating warnings
by Kobus Myburgh 10 Sep '05

10 Sep '05

Please post these questions to the support list. Post your query to the support list and I will help you with this. Kobus >>> patkins(a)directpartners.com 9/10/2005 12:46:19 AM >>> My site requires that all user are Authenticated and I'm looking for a way to stop Anonymous users from even reaching a page and generating the following error. access denied 2005-09-09 15:39 node/755 denied access. Anonymous Is there a build method for doing this with Drupal or do I need to create something custom? Thanks for you help! Access Manager: This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Dissemination, distribution or copying of this e-mail or the information herein by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is prohibited. If you have received this e-mail in error, please immediately notify us by calling the Help Desk at (310) 482-4200.

1 0

Re: [drupal-devel] Permission Denied Errors
by Kobus Myburgh 10 Sep '05

10 Sep '05

Hi, I remember that you are using Windows, like me. I had to remove .htaccess file to fix this problem. These reports belong to the support list. Please post them there in the future. Kobus >>> patkins(a)directpartners.com 9/10/2005 12:12:25 AM >>> I'm getting "permission denied "from drupal on pages that I refresh multiple times. Sometimes it works sometimes it doesn't. Anyone have any thoughts? -p

1 0

Re: [drupal-devel] Drupal on Windows...having issues
by Kobus Myburgh 10 Sep '05

10 Sep '05

Hi, I have no benchmarks, but I am running several Drupal sites on Windows and several on Linux. The only issue I had was that I had to remove the .htaccess file that comes with Drupal, but otherwise none. Speed also okay. Regards, Kobus >>> patkins(a)directpartners.com 9/9/2005 6:48:46 PM >>> I'm a Linux guy trying to run Drupal on Windows 2k Server. Not a choice I have control over. I'm having some serious performance issues and need some help. Are there benchmarks for Drupal on various platforms? If so where would I find that information. Anyone know if Drupal can run on windows with decent performance? Anything will help. Cheers!

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Zed Pobre 10 Sep '05

10 Sep '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: Jeremy(a)kerneltrap.org Reported by: Jeremy(a)kerneltrap.org Updated by: Zed Pobre Status: patch (code needs review) This patch is against HEAD? It doesn't want to apply to my 2.6.3 comment.module. Zed Pobre 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. ------------------------------------------------------------------------ Wed, 17 Aug 2005 19:58:02 +0000 : breyten 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? ------------------------------------------------------------------------ Thu, 18 Aug 2005 03:20:02 +0000 : Jeremy(a)kerneltrap.org > Well, this patch is definitely better than what we have, and would save some spam for sure. It is continuing to work very well on my site, which seems to be under nearly perpetual spam attacks from multiple sources. > 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. The catch is: the key has to be something unique to the server, not guessable or learnable from the outside Simply storing the hash data in the session alone is not enough, as then the spammer could create any random data and store it in the session. That said, the hash could be generated off something other than the text of the comment as it is now, so that a preview is not required. I'll look at doing something like that and submit another patch. > 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. nid and pid alone are worthless, as they are easy to learn. The pid can always be 0 (spam is rarely attached to a pre-existing comment). The nid is obtained in the path of where the spam is being posted. The solution is a "private-key", which is what my patch adds. Then sure, hash the private key plus the nid and the pid, and you've got enough protection to prevent most spammers. To make it even more secure, automatic rey-keying could be easily accomplished. ------------------------------------------------------------------------ Thu, 18 Aug 2005 04:09:10 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_15.patch (1.18 KB) The attached patch: 1) gets rid of the _cron() hook 2) no longer requires that comments be previewed Prior to this patch, comment spammers were able to send data to a Drupal server acting as though they'd filled out a comment form and pressed submit. As they didn't actually use the form, they could submit spam comments at an obscene rate. With this patch, comment spammers will have to actually load the form, enter text, and press submit. Yes, that can still be automated, but it takes much more work and slows them down, as they have to wait for the entry form to load each time. Unfortunately a spammer could manually submit one comment, then re-use that same session info over and over to attach repeated spam comments to the same node. Such an attempt would be detected and blocked by the spam module if enabled, but again such a session re-use attack could be done without loading the form each time. Fortunately there is much less gain for a spammer to submit 100 spam comments on the same page, versus submitting 100 spam comments each on a different page as they do now. Ideas to improve upon this concept include: - re-key every day or week, changing the private key regularly to be sure it couldn't ever be permanently cracked - add a key table, and generate a unique key for every comment form. essentially, upon comment form creation generate a random key which is stored both in a database table and in the session. when a comment is submited, look for the key from the session in the database table, if a match is found delete it from the database table and post the comment. this would prevent session re-use, but adds overhead. i don't know if it's worth it, perhaps as an external module if the hooks were available. ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:55:11 +0000 : drumm <?php form_set_error('error', t('Validation error, please be sure cookies are enabled on your browser.')); ?> form_set_error [2]()'s fist argument should be the name of a form field, not 'error.' Using (..., 'error') would be better in this case. And the actual message needs work. Since this is a hidden field I don't think it has anything to do with cookies. [2] http://drupaldocs.org/api/head/function/form_set_error ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:56:41 +0000 : drumm The unclosed link in my last update was supposed to say drupal_set_message(..., 'error') ------------------------------------------------------------------------ Sat, 20 Aug 2005 16:00:15 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_16.patch (1.11 KB) drupal_set_message(..., 'error') isn't sufficient to prevent the comment from being posted. I have instead updated the patch to set the error on the hidden 'token' form field. I have updated the message to read: "Unable to validate your comment, please try again. If this error persists, please contact the site administrator." If you don't like the error message, better suggestions are welcome. ------------------------------------------------------------------------ Fri, 09 Sep 2005 03:16:06 +0000 : Jeremy(a)kerneltrap.org Any feedback on this patch? I have been running it on my website for a couple of weeks, and it has completely stopped the most persistent auto-spam scripts that had been posting poker type comments constantly to my site.

1 0