- Development - lists.drupal.org

Re: [drupal-devel] [contributions:weitzman] /modules/devel pgprefix.sh
by Dries Buytaert 01 Jun '05

01 Jun '05

On 01 Jun 2005, at 05:29, drupal-cvs(a)drupal.org wrote: > User: weitzman Branch: HEAD Date: Wed, 01 Jun 2005 03:29:27 > +0000 > > Added files: > /modules/devel pgprefix.sh > > Log message: > #23340 add a pgsql prefix script The prefix.sh script in HEAD (and DRUPAL-4-6, not Drupal 4.6.0) should work with PostgreSQL. -- Dries Buytaert :: http://www.buytaert.net/

1 0

[drupal-devel] [bug] Enable/disable user picture option should not exist in two places
by Steven 01 Jun '05

01 Jun '05

Issue status update for http://drupal.org/node/23310 Project: Drupal Version: 4.6.0 Component: user.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: factoryjoe Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/picture_0.patch (1.22 KB) Ah, it seems we prefer to disable the toggles rather than hide them. It is probably for the best (I've always read you should disable options if they can be enabled by the user in this session). I had to generalize the check for search.module a bit, the code is cleaner now. Steven Previous comments: ------------------------------------------------------------------------ May 21, 2005 - 01:47 : factoryjoe Because you can enable/disable user pictures in the PHPTemplate config pages, that option should not exist under admin/users. And if PHPTemplate shouldn't handle this option, it should be removed from PHPTemplate's config screens. This option should be set in one place, and one place only. ------------------------------------------------------------------------ June 1, 2005 - 06:47 : Steven Attachment: http://drupal.org/files/issues/picture.patch (1.07 KB) I disagree... the option in admin/users controls whether users can upload a picture. This image is always displayed in their profile and this is a user.module feature. The PHPTemplate options on the other hand control whether pictures are displayed next to nodes resp. comments. This is a theme feature. The two are different and complement each other. Sometimes you may want users to have pictures, but not have them displayed next to each post. In that case you could even let users upload larger pictures of themselves as they don't need to fit snugly next to text. However you are partially right: it does not make sense to show the PHPTemplate options if user pictures are not allowed, so we should only make them appear in the right situation. I've attached a patch for this.

1 0

[drupal-devel] [feature] Replace $_SERVER["REMOTE_ADDR"] with a function which understands X-Forwarded-For
by Steven 01 Jun '05

01 Jun '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: Steven Status: patch In Drupal we tend to store the original information in the db, escaping or formatting on output as needed. All places where the hostname is output should be okay after some recent changes. Steven Previous comments: ------------------------------------------------------------------------ April 14, 2005 - 01: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 - 23: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 - 10: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 - 06: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 - 06: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. ------------------------------------------------------------------------ May 27, 2005 - 00:58 : degerrit 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…

1 0

[drupal-devel] [bug] Statistic information about node reads is shown to unprivileged users
by Steven 01 Jun '05

01 Jun '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: Robin Monks Reported by: massabob Updated by: Steven Status: patch I'm not sure about this patch: often, read counts are shown directly on the site. But if the permission for viewing the counts is the same as the permission for accessing the administrator's detailed logs, then you wouldn't give that to everyone. There is already an option to choose whether counts are displayed. Perhaps we could change that to "No" "For priviledged users" "For everyone". In last case it acts like it is now, it the second case it requires "access statistics" permission. What do you think? Steven Previous comments: ------------------------------------------------------------------------ May 11, 2005 - 09: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 - 03:44 : rbarreca Should read user_access('access statistics') not user_access('display statistics'). ------------------------------------------------------------------------ May 27, 2005 - 13:19 : Robin Monks Attachment: http://drupal.org/files/issues/user.access.stats.patch (730 bytes) And here that is in patch form. Robin

1 0

[drupal-devel] [feature] move check_query() into the database interface files
by Steven 01 Jun '05

01 Jun '05

Issue status update for http://drupal.org/node/13180 Project: Drupal Version: cvs Component: database system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: Steven Status: patch Actually, Drupal does nothing to ensure the database character set is set to UTF-8 (can it?). Thus, escaping with mysql_real_escape_string() is not advised. Steven Previous comments: ------------------------------------------------------------------------ November 19, 2004 - 21:35 : chx Attachment: http://drupal.org/files/issues/check_output_patch.txt (1.54 KB) The check_query documentation says 'Prepare user input for use in a database query, preventing SQL injection attacks.' But the code itself is a simple addslashes. This is simply not correct -- for mysql, yes, but the world is not just mysql. There are _escape_string functions for that: mysql_escape_string, pg_escape_string, sqlite_escape_string... Read the manual for pg_escape_string: "Use of this function is recommended instead of addslashes()." Or read sqlite_escape_string: "addslashes() should NOT be used to quote your strings for SQLite queries; it will lead to strange results when retrieving your data." Ooooops. So I propose moving check_query() declaration from bootstrap.inc to database.*.inc. I've already written a letter to the devel list about this, but drumm said "code is better than talk" so here is a patch. At this moment, this does absolutely nothing, only opens the possibility to use the aforementioned escape_string functions. But database.sqlite.inc simply will not work without this change. ------------------------------------------------------------------------ November 19, 2004 - 21:42 : drumm Looks straightfoward enough. Is there any reason to not put in pg_escape_string() now? ------------------------------------------------------------------------ November 19, 2004 - 21:47 : chx Actually, yes. Read on the mentioned manual page: "Note: This function requires PostgreSQL 7.2 or later." I have no idea how the check it. There is a pg_version() but I will leave this postgres business to someone who has a working postgres install to code and check this thing. ------------------------------------------------------------------------ November 19, 2004 - 22:58 : Chris Johnson PostgreSQL 7.2 was released February 4, 2002. Major production releases 73. and 7.4, along with many point releases, have been issued since then. Current production is 7.4.6 and 8.0 is in the 4th revision of beta. It seems pretty safe to assume that any Drupal installation using PostgreSQL would have version 7.2 or later. Document if a user encounters errors with PostgreSQL that they need to check the version is 7.2 or later. ------------------------------------------------------------------------ November 20, 2004 - 15:42 : Dries +1 I suggest creating a db_escape_string() function, and removing check_query() all together. ------------------------------------------------------------------------ November 20, 2004 - 15:55 : Goba +1 on Dries ------------------------------------------------------------------------ November 20, 2004 - 19:32 : chx Attachment: http://drupal.org/files/issues/check_output_patch_0.txt (1.6 KB) db_escape_string sounds find to me, it's really a better name, thanks Dries. But removing check_query? I'd suggest deprecating it and removing it later. I am afraid It would break a lot of custom modules. Here is a slightly modified patch with pg_escape_string included and documented. ------------------------------------------------------------------------ November 20, 2004 - 19:43 : killes(a)www.drop.org We were never afraid of breakign things for the good of the project. Please don't change this policy. ------------------------------------------------------------------------ November 20, 2004 - 20:26 : chx Attachment: http://drupal.org/files/issues/db_escape_string.txt (20.87 KB) Let's do it. ------------------------------------------------------------------------ November 20, 2004 - 20:29 : chx Attachment: http://drupal.org/files/issues/db_escape_string_0.txt (20.88 KB) Something funny happened to the PostgreSQL db_escape_string comment and I have missed it, sorry. Reposting the patch. ------------------------------------------------------------------------ November 21, 2004 - 10:29 : Dries I committed the patch. Thanks chx. Keep it coming. I've also updated the upgrade notes at http://drupal.org/node/12347. I'll update some contributed modules unless someone objects. ------------------------------------------------------------------------ November 21, 2004 - 13:27 : chx Attachment: http://drupal.org/files/issues/mysql_escape_patch.txt (572 bytes) Keep it coming? Well, let's see the MySQL case... It's far from being trivial, and I expect some controversy over it. ------------------------------------------------------------------------ March 10, 2005 - 01:36 : chx Attachment: http://drupal.org/files/issues/mysql_escape_patch_0.txt (573 bytes) OK, here is mysql version a bit corrected, this should get into 4.6 -- addslashes is not a correct way any more. ------------------------------------------------------------------------ March 10, 2005 - 20:29 : Dries Can we read up on that somewhere? ------------------------------------------------------------------------ March 10, 2005 - 20:37 : Goba Dries, sure: http://php.net/mysql_escape_string [1] and http://php.net/mysql_real_escape_string [2]. The latter also uses the connections charset settings to properly escape, so it is the best (but is not available in old PHP versions). The addslashes page user notes also have more information [3]. [1] http://php.net/mysql_escape_string [2] http://php.net/mysql_real_escape_string [3] http://hu.php.net/addslashes#20013 ------------------------------------------------------------------------ March 12, 2005 - 10:50 : Dries addslashes() has been working for ages (and is simpler code-wise). I guess the mysql_ functions are slightly more secure or not even that? That aside, I'm not really a big fan of version_compare()s. Also note that the patch violates Drupal's coding standards (again) but that's easy enough to fix. ------------------------------------------------------------------------ March 13, 2005 - 21:49 : chx I can only repeat what Goba said: . mysql_real_escape_string uses the connections charset settings to properly escape, so it is the best (but is not available in old PHP versions).

1 0

[drupal-devel] [bug] Menu cache prohibiting access for users
by Steven 01 Jun '05

01 Jun '05

Issue status update for http://drupal.org/node/24034 Project: Drupal Version: 4.6.0 Component: menu system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: nysus Updated by: Steven -Status: active +Status: patch Attachment: http://drupal.org/files/issues/user.menu.cache.patch (1.62 KB) Patch attached. Steven Previous comments: ------------------------------------------------------------------------ June 1, 2005 - 02:05 : nysus See http://drupal.org/node/24019 for description of the problem.

1 0

[drupal-devel] [feature] Don't promote forum topics by default
by Steven 31 May '05

31 May '05

Issue status update for http://drupal.org/node/24003 Project: Drupal Version: cvs Component: forum.module Category: feature requests Priority: normal Assigned to: Steven Reported by: Steven Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/forumpromote.patch (1.14 KB) One side effect of the node system is that all the default node flags (promote, revisions, etc) have only one default setting for all node types. The most meaningless result of that IMO is that forum topics are promoted to the front page by default. This is probably one of the most confusing things people run into when setting up a Drupal forum. The attached patch adds a row to the variable table to force forum topics to not be promoted by default. This will only affect new installations. Patch is for CVS, but IMO it should go in 4.6 too. Steven

1 2

[drupal-devel] Seeking maintainer of webform.module
by Syscrusher 31 May '05

31 May '05

Greetings! I've tried contacting the person listed with the project, but never got any response, so I'll post more broadly here in case that person has moved on to other things. I need to reach the maintainer of the "webform" module as soon as possible, because I have some contributed patches to offer. My company needed to use this module internally, and needed some enhancements. So I got paid to write them, the company gets to use them, and per my company's wonderful Open Source policy, I get to contribute them back to the community under GPL. Everyone wins. What I have added to webform.module is: * Additional flexibility in setting the "Subject" line for emails of submitted forms, including a template-built subject line that can optionally include values of one or more fields from the submitted form. * Ability for the administrator to force the "From" address of generated emails to be the email address from the authenticated Drupal user, instead of an address the user enters into the form. * A new field type "select sql" that allows an arbitrary SQL query to return the list of options for list, checkbox, or radio fields. This allows, for example, a selection list that includes things like "all of the titles of the user's last N nodes" or similar site-defined queries. * A new field type "select php" that calls an arbitrary PHP function to get the list of options for list, checkbox, or radio fields. I'm using this feature to allow users to select a project number from a completely separate project tracking system The code is done, tested, and working with one exception: I need to add access checks so that editing the two new field types is restricted only to users who can create PHP-content nodes, to prevent a security hole. This isn't an issue at our company, because the "create webforms" access is already tightly restricted, but it is a potential issue in the broader community. The patches I have are against a very recent (< 1 week old) copy of the 4.6 branch for this module. So I just need to know whom to contact to submit the patch. Anyone care to step forward or offer the name and email of the current maintainer? Any assistance appreciated. Kind regards, Scott -- ------------------------------------------------------------------------------- Scott Courtney Drupal user name: "syscrusher" http://drupal.org/user/9184 scott at 4th dot com Drupal projects: http://drupal.org/project/user/9184 Sandbox: http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/syscrusher

1 0

[drupal-devel] [bug] menu error line 974 with MBstring
by mark.stephens 31 May '05

31 May '05

Issue status update for http://drupal.org/node/22026 Project: Drupal Version: cvs Component: menu system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: hendler Updated by: mark.stephens Status: patch I was having this problem with a clean install of drupal 4.6.0. I had mbstrings enabled in PHP. I turned it off, and the problem went away immediately. Within mbstrings, I had: mbstring.language = English mbstring.internal_encoding = UTF-8 mbstring.http_output = auto mbstring.func_overload = 7 PHP 4.3.11. Mark mark.stephens Previous comments: ------------------------------------------------------------------------ May 5, 2005 - 00:15 : hendler Menu.inc on line 974 the do {}while() causes an error if $parent is empty because strrpos, (I have mbstring enabled) may be null to fix my problem I have this if statement wrapping it, which I understand is logically equivlent to just doing a while loop. if ($parent){ do { $parent = substr($parent, 0, strrpos($parent, '/')); } while ($parent && !array_key_exists($parent, $_menu['path index'])); } This happens just after a new install. ------------------------------------------------------------------------ May 5, 2005 - 23:43 : Robin Monks Attachment: http://drupal.org/files/issues/menu.inc_0.patch (908 bytes) We apparently don't check if the object exists before we work on it. Robin ------------------------------------------------------------------------ May 6, 2005 - 09:43 : Dries I wonder how I can reproduce this problem. Maybe we are fixing the symptom, not the cause.

1 0

[drupal-devel] Legal Support
by Andre Molnar 31 May '05

31 May '05

Sorry for the cross post, Do we have any members in the community that are lawyers. Occasionally we have questions pop up regarding GPL and Licensing compatibilities - and there are bound to be questions about the legality of some of the documentation. I hope to continue to help coordinate the marketing documentation, but I'm sure there are times when legal questions will arise about making certain claims or citing names in marketing documents. Is there anybody that we have that we can turn to for these kinds of questions? For example, Joshuwa - who has been doing some amazing work on the print ad - asked: "Name-dropping is an excellent idea, especially big names like 'Yahoo!', but I am almost certain you'd need permission from Yahoo! before including their name/identity in an ad. I'm not 100% sure, but might be something to consider." I'm 99% sure that we wouldn't need permission - stating a fact is a fact and the fact is that Yahoo is using drupal internally - but then again I'm not a lawyer. andre

2 1