May 2005 - Development - lists.drupal.org

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

27 May '05

Issue status update for http://drupal.org/node/22565 Project: Drupal Version: 4.6.0 Component: statistics.module Category: bug reports Priority: normal -Assigned to: Anonymous +Assigned to: Robin Monks Reported by: massabob Updated by: Robin Monks -Status: active +Status: patch Attachment: http://drupal.org/files/issues/user.access.stats.patch (730 bytes) And here that is in patch form. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ May 11, 2005 - 07:51 : massabob Information about '%count reads' in node's footer is shown to unprivileged users. I suggest that function statistics_link in statistics.module should be corrected in this way: // Original function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0)) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } // Fixed function statistics_link($type, $node = 0, $main = 0) { global $id; $links = array(); if ($type != 'comment' && variable_get('statistics_display_counter', 0) && user_access('display statistics')) { $statistics = statistics_get($node->nid); if ($statistics) { $links[] = format_plural($statistics['totalcount'], '1 read', '%count reads'); } } return $links; } The only change is in "&& user_access('display statistics')" on line 98. ------------------------------------------------------------------------ May 27, 2005 - 01:44 : rbarreca Should read user_access('access statistics') not user_access('display statistics').

1 0

[drupal-devel] [bug] New pager code broken taxonomy pagers
by Morbus Iff 27 May '05

27 May '05

Issue status update for http://drupal.org/node/23687 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: Morbus Iff Status: patch Which part of comment.module? I hope you don't think the crazy "thread 1.1.2/, 1.2" stuff is /better/. Morbus Iff Previous comments: ------------------------------------------------------------------------ May 26, 2005 - 14:44 : Morbus Iff I've not yet looked into it, but I doubt I'll be able to fix it (the original taxonomy/term pager code was a hack that moshe taught me about). In a nutshell, though, the recent pager cleanup in head has broken the term pagers for vocabularies. ------------------------------------------------------------------------ May 26, 2005 - 15:21 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_23687_taxopager.patch (1.88 KB) Patch attached. ------------------------------------------------------------------------ May 27, 2005 - 02:05 : Steven Why can't this code use pager_query() and query the database directly? If free tagging vocabularies can get so big, it seems a very inefficient thing to fetch the entire list. ------------------------------------------------------------------------ May 27, 2005 - 02:05 : Steven Note: if you need to fetch a list of tags ordered by hierarchy in one query, you should look at how comment.module does it.

1 0

[drupal-devel] More flexible user profile page - blocks to the rescue
by Moshe Weitzman 27 May '05

27 May '05

So I'm looking at how to make the user profile page prettier and more useful. I'm using the various social networking sites for ideas. Here are 3 sample pages in case you are curious: http://www.friendster.com/user.php?uid=1856275 http://360.yahoo.com/profile-9lciejI3aafX1stHPoIRNmkmv4EowQ-- http://levistus.1up.com/ The user profile page is actually quite simple. The rendering function [theme_user_profile()] accepts an associative array where a key is a 'category name' and a value is a hunk of HTML. The categories are defined by the modules which are active. The order of the categories is defined by alphabetical order of module names. Note that any module can inject content onto the profile page, not just profile.module. This is good. In order to make this more flexible, we want to give the admin the choice of which items to show and where to show them. And perhaps give the user the ability to customize his profile page. It turns out that we already have a great system for managing this - blocks. Ideally, I think we could reuse the block admin page for profile page admin. There, admin will decide which 'categories' appear where, and even reuse logic like whether a category may be enabled/disabled by a user. We would not use the feature of blocks to turn on and off by path since the profile page only has one path. I'm interested in your thoughts about how this could be implemented from a UI perspective. My inclination is to add a new page for profile admin which reuses code from block.module. I'm not inclined to move block code into a block.inc, especially because block.module is required. I'd like to reuse the blocks table. I'm not sure how this fits into the 'block regions' work in progress. It would be nice to have more regions than left/right. Even without more regions, this would be useful than our current 'single column' model. Perhaps chx or nedjo could comment on this. Also, I'm not sure that grouping profile items into 'categories' is useful in this model. We might be better off copying hook_block() and letting modules declare their own blocks instead of fitting their items into arbitrary categories. Lastly, there is a chance that all this work inhibits sites which implement their own theme_user_profile(). They are currently able to do whatever they want with categories but I'm not so sure we can preserve this using the block paradigm. Feedback welcome. I'm not planning on starting code on this immediately. In fact, I'd be pleased if someone else wanted to run with this.

2 1

Re: [drupal-devel] [drupal:unconed] /includes common.inc
by Stefan Nagtegaal 27 May '05

27 May '05

Op 27-mei-2005, om 1:39 heeft drupal-devel(a)drupal.org het volgende geschreven: > User: unconed Branch: HEAD Date: Thu, 26 May 2005 23:39:25 +0000 > > Modified files: > /includes common.inc > > Log message: > - #23700: Fix some broken t()'s in common.inc > > Links: > http://cvs.drupal.org/diff.php?path=drupal/includes/ > common.inc&old=1.448&new=1.449 > > -- > [ Drupal cvs list | http://list.drupal.org/ ] Ehmmm.... Maybe it is me, but didn't we agreed that we use meaningfull variable names? So: - watchdog('php', t("Could not convert XML encoding '%s' to UTF-8.", $encoding), WATCHDOG_WARNING); + watchdog('php', t("Could not convert XML encoding '%s' to UTF-8.", array('%s' => $encoding)), WATCHDOG_WARNING); and: - watchdog('php', t("Unsupported encoding '%s'. Please install iconv, GNU recode or mbstring for PHP.", $encoding), WATCHDOG_ERROR); + watchdog('php', t("Unsupported encoding '%s'. Please install iconv, GNU recode or mbstring for PHP.", array('%s' => $encoding)), WATCHDOG_ERROR); Had to be something like: - watchdog('php', t("Could not convert XML encoding '%s' to UTF-8.", $encoding), WATCHDOG_WARNING); + watchdog('php', t("Could not convert XML encoding '%encoding' to UTF-8.", array('%encoding' => $encoding)), WATCHDOG_WARNING); - watchdog('php', t("Unsupported encoding '%s'. Please install iconv, GNU recode or mbstring for PHP.", $encoding), WATCHDOG_ERROR); + watchdog('php', t("Unsupported encoding '%encoding'. Please install iconv, GNU recode or mbstring for PHP.", array('%encoding' => $encoding)), WATCHDOG_ERROR); If yes, I'll provide a patch to correct this.. Stefan..

2 2

[drupal-devel] Admin help longs to be read by skilled Drupal user
by Kieran Lal 27 May '05

27 May '05

Howdy, 203 of you completed the Drupal documentation survey and told us that the number one thing you wanted was module documentation. The goal of this admin help documentation is: 1) Provide documentation for Drupal modules. 2) Improve the usability of the admin help. 3) Improve the documentation teams ability to work with admin help by single sourcing it from the documentation handbook. Here is the documentation for Drupal core:http://drupal.org/handbook/ modules Here is the documentation for some of Drupal contributions: http:// drupal.org/handbook/modules/contributions The handbook module pages start with executive summaries that are also admin help. Pretty clever, don't you think. The handbooks are currently under development, stay tuned for more improvements. It would be greatly appreciated if knowledgeable Drupal users could read some of them and report back on where they are wrong. If you need a site to confirm this documentation, create an account on: http://demo.civicspacelabs.org/home and send me a link to your profile. I'll make sure you get the access you need to confirm the documentation is accurate. Thanks for you help, Kieran

1 0

[drupal-devel] [bug] New pager code broken taxonomy pagers
by Steven 27 May '05

27 May '05

Issue status update for http://drupal.org/node/23687 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: Steven Status: patch Note: if you need to fetch a list of tags ordered by hierarchy in one query, you should look at how comment.module does it. Steven Previous comments: ------------------------------------------------------------------------ May 26, 2005 - 21:44 : Morbus Iff I've not yet looked into it, but I doubt I'll be able to fix it (the original taxonomy/term pager code was a hack that moshe taught me about). In a nutshell, though, the recent pager cleanup in head has broken the term pagers for vocabularies. ------------------------------------------------------------------------ May 26, 2005 - 22:21 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_23687_taxopager.patch (1.88 KB) Patch attached. ------------------------------------------------------------------------ May 27, 2005 - 09:05 : Steven Why can't this code use pager_query() and query the database directly? If free tagging vocabularies can get so big, it seems a very inefficient thing to fetch the entire list.

1 0

[drupal-devel] [bug] New pager code broken taxonomy pagers
by Steven 27 May '05

27 May '05

Issue status update for http://drupal.org/node/23687 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: Steven Status: patch Why can't this code use pager_query() and query the database directly? If free tagging vocabularies can get so big, it seems a very inefficient thing to fetch the entire list. Steven Previous comments: ------------------------------------------------------------------------ May 26, 2005 - 21:44 : Morbus Iff I've not yet looked into it, but I doubt I'll be able to fix it (the original taxonomy/term pager code was a hack that moshe taught me about). In a nutshell, though, the recent pager cleanup in head has broken the term pagers for vocabularies. ------------------------------------------------------------------------ May 26, 2005 - 22:21 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_23687_taxopager.patch (1.88 KB) Patch attached.

1 0

[drupal-devel] Drupal.org cache statistics
by Dries Buytaert 27 May '05

27 May '05

Hello world, november last year, I profiled drupal.org's cache observations. Yesterday, Moshe asked to profile it again so we could evaluate the usefulness of Jeremy's "loose caching" mechanism. The past 20 hours, I logged 93.000 unique page requests using the patch at http://buytaert.net/temporary/cache-statistics.patch. Loose caching was enabled. Results: 1. Last year we found that authenticated users were responsible for 15,8 % of all page views. A year later, we see that authenticated users are responsible for 14,9% of all page views. 2. Last year we found that only 27.9% of the page requests actually benefited from the cache system. That is, for more than 2/3th of the page requests, we had to generate a page dynamically. A year later, using "loose caching" rather than "strict caching", we see that 30,7% of the page requests benefit from the cache system. Read: we still have a lot of page cache misses! :( 3. Last year, we found that the cache got flushed once every 207 page requests. A year later, we observe that the cache got flushed once every 190 page requests. We conclude that: 1. Loose caching does not significantly -- or not necessarily -- improve the behavior of drupal.org's page cache (though I'd like to believe that it does when there are sudden traffic spikes/bursts). 2. When writing code, we can NOT assume that a page will benefit from being cached. -- Dries Buytaert :: http://www.buytaert.net/

7 9

[drupal-devel] [feature] Replace $_SERVER["REMOTE_ADDR"] with a function which understands X-Forwarded-For
by degerrit 27 May '05

27 May '05

Issue status update for http://drupal.org/node/20471 Project: Drupal Version: 4.6.0 Component: base system Category: feature requests Priority: normal Assigned to: degerrit Reported by: degerrit Updated by: degerrit Status: patch I will re-write the patch with Dries' suggestions, I haven't had much time this week. I found some interesting examples of scripts doing roughly the same, none of them seem to do input checking (which I hadn't thought of either, but is obviously very needed!). Good thing there are people like Dries around, eh! There's also another possible header: HTTP_CLIENT_IP, I've never seen it but then I don't log all my clients' headers either ;-) http://be2.php.net/getenv (the 2nd comment at this time) http://php.planetmirror.com/manual/en/function.getallheaders.php http://ip-to-country.webhosting.info/node/view/212 http://dev.wp-plugins.org/file/wp-contact-form/trunk/wp-contactform.php?rev… degerrit Previous comments: ------------------------------------------------------------------------ April 14, 2005 - 00:42 : degerrit In several modules, replace $_SERVER["REMOTE_ADDR"] with a function which understands X-Forwarded-For. Proxy servers often (sometimes, always?) insert "X-Forwarded-For" headers which contains the "real" client IP address. I think it is useful to log the client IP address in that situation. This little snippet works for me, and I'd suggest something similar be made into a function? $headers = apache_request_headers(); if (array_key_exists('X-Forwarded-For', $headers)){ $hostname=$headers['X-Forwarded-For'] . ' via ' . $_SERVER["REMOTE_ADDR"]; } else { $hostname=$_SERVER["REMOTE_ADDR"]; } I can make a patch proposal if people like the idea. ------------------------------------------------------------------------ May 21, 2005 - 22:44 : degerrit Attachment: http://drupal.org/files/issues/remote_addr.diff (6.07 KB) Here's my proposal for showing user IP addresses behind proxy servers, especially in log files. It adds a function remote_addr() and replaces all the uses of $_SERVER["REMOTE_ADDR"] with that function. This is my first patch for Drupal, don't be too harsh one me :-) A few notes: - I patched session.inc because $_SERVER["REMOTE_ADDR"] is also called in session.inc and common.inc is not available at that time it seems - maybe I replaced too vigorously in common.inc and sessions.inc, it works for me though - maybe it should be renamed http_remote_addr or apache_remote_addr ------------------------------------------------------------------------ May 22, 2005 - 09:37 : Dries Couple comments: Drupal supports non-Apache webservers: apache_request_headers() might not always be available. Does this patch need input checking? Depends on how/when/where the remote address data is used. Fact is that we don't want people to inject XSS-stuff through HTTP headers. Make sure to double-check this. I'd rename the function to remote_address(). We usually don't abbreviate words like 'address'. The word 'via' can't be translated. We write } else { and not } else {Just thinking up loud. Is it easy to fake or spoof X-Forwarded-For-headers? If it is, it would be easy to trick Drupal (eg. in the statistics.module's log pages we group by remote address). For the watchdog it doesn't matter, but for the statistics module it does. Maybe we need a toggle so remote_address() can be told not to return the 'true IP address'. ------------------------------------------------------------------------ May 25, 2005 - 05:37 : Steven X-Forwarded-For can be spoofed without any trouble. Just connect and send the fake header. I don't think we should pay attention to it. If we do, we need to also store the original IP address. Perhaps we could do "real.ip.#.# (forwarded.ip.#.#)" ------------------------------------------------------------------------ May 25, 2005 - 05:39 : Steven Sorry didn't look at the patch carefully enough. Yes, we need XSS protection: in practice, we always strore remote_addr() in the hostname column in the db. Wherever that column is displayed, it needs to be check_plain()ed.

1 0

[drupal-devel] [bug] broken t()s in common.inc
by killes 27 May '05

27 May '05

Issue status update for http://drupal.org/node/23700 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org Status: patch Attachment: http://drupal.org/files/issues/broken-t.patch (1.1 KB) there are two broken t() function in common.inc.The patch shoul dalso be applied to 4.6 (will make a new one if it shoul dnot apply). killes(a)www.drop.org

1 0