March 2005 - Development - lists.drupal.org

[drupal-devel] [feature] Trim username at login
by Bèr Kessels 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/11791 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: kps Updated by: Bèr Kessels Status: patch "apart from Ber everyone liked it", so here a short comment: I still beleive modifying what a user inserts should never be modified. But the current situation is far worse, so I guess it *gets a +1* from me now too :). Bèr Kessels Previous comments: ------------------------------------------------------------------------ October 20, 2004 - 18:29 : kps They create an account. They get a password by email. They select the password plus the preceding blank or following newline. They paste it into the 'Password' box. They can't log in. They write to me complaining. I get tired of complaints and patch user.module to strip leading and trailing blanks. ------------------------------------------------------------------------ October 20, 2004 - 18:39 : kps Attachment: http://drupal.org/files/issues/user.module_0.patch (766 bytes) I claim I'm not *really* dumber myself. Drupal timed out on me.... ------------------------------------------------------------------------ October 20, 2004 - 19:02 : moshe weitzman makes sense to me ------------------------------------------------------------------------ October 20, 2004 - 22:54 : rkendall +1 FWIW - I could see this being useful, and don't really see any practical drawbacks. I does smell of hack, but is that really an issue? I mean, it might not be the 'done' thing to mess with passwords, however, I can't imagine any regular user intentionally putting leading or trailing whitespace on a password, but I can imagine it being done accidentally fairly often (either when setting a password, or when logging in). To be consistent, it would make sense to trim passwords when setting them as well. ------------------------------------------------------------------------ October 21, 2004 - 00:09 : Bèr Kessels -1 from me. I think we should nstart meddling with passwords by trim()ing them. If you have dumb users, you should fix it in the mail that is send to them. That is really easy. For example: add a word after the passwords: your password is WKDKAFAJ34 please mind capital letters. or so. Bèr ------------------------------------------------------------------------ October 21, 2004 - 00:36 : Steven I don't agree with Bèr... copy/pasting is a fiddly business, especially because when pasting a password all you see is asterisks and you don't notice if there is an extra character. Trimming the password won't hurt, I very much doubt that there are people who consciously use a space at the beginning or end of their password. +1 on trimming, it is a usability improvement. ------------------------------------------------------------------------ October 21, 2004 - 00:51 : chx +1 here. Bèr, if you write a sentence around it, you can still copy-paste the whitespace before and after it. ------------------------------------------------------------------------ October 21, 2004 - 02:18 : Uwe Hermann I'm unsure if I really like this, but if this really gets applied, please make it a configuration option. Do not hardcode it for all Drupal installations. Thanks, Uwe. ------------------------------------------------------------------------ October 21, 2004 - 06:06 : robertDouglass +1 I can confirm from my logs that the typical user bungles initial login 1-4 times, with each bungled attempt making an all-out failure more likely. Not only am I in favor of trimming, I am much in favor of investigating other means of initial password assignment like on the initial register form or by generating a unique URL that gets mailed and only has to be clicked or pasted into the browser address bar. Sorry if those alternatives have already been widely discussed here. ------------------------------------------------------------------------ October 21, 2004 - 07:21 : stefan nagtegaal I am all for trimming the spaces in front of the username and after it, but I am absoklutely against another option for such thing. IMO this is only a usability improvement and it is not needed to make it a configurable behaviour.. We have enough options already, and if you'll ask me I'll tell you that we need less options instead of more.. ------------------------------------------------------------------------ October 21, 2004 - 10:58 : Bèr Kessels Admitted: no one will conciously add spaces to his or her passwords. So i will pull out my -1 hereby. but -1 for making ot an option. As steef syis: its useability. I still stick to the -1 for applying this specific patch. I beleive that we should be 1) consitant, and strip /all/ password whitespace. 2) use drupal_set_message() to warn people when whitespace was stripped. And so to educate users to be aware of whitespace when copying passwords. ------------------------------------------------------------------------ October 21, 2004 - 12:39 : kps My proposed patch also strips white space when the user changes the password, so it's not possible to create an unusable password. I agree that a warning message would be a good idea. ------------------------------------------------------------------------ October 21, 2004 - 17:10 : Chris Johnson Part of the problem with users copying and pasting extraneous whitespace around passwords (or userrnames) is the inconsistent behavior in GUI windowing environments. That is to say, in some applications in some GUIs, double-clicking on a word will copy the word and its surrounding white space, for example. In others, it will not. One might think it would be visibly obvious that the surrounding white space was included in the copy operation, but that's not so. The words might be displayed in a very small, proportional font (and further might be justified or kerned in unpredictable fashion) which make it hard to see just where the highlighted copied text begins and ends. This is under the control of the application and the user. Or, the user might think logically that since all he or she wanted was the word, and likewise that "whitespace" is irrelevant, the user may assume that what was copied was only the desired word even if they can visibly see that the adjacent spaces are highlighted. I'm well aware of this behavior myself and even I sometimes get tripped up when copying and pasting bits of data here and there by the occasional undesired white space. My vote would be to always trim leading and trailing white space, and to document in the help that such white space is not valid in passwords. ------------------------------------------------------------------------ March 13, 2005 - 19:25 : killes(a)www.drop.org The patch still applies. Apart from Ber everybody liked it. I also like it.

1 0

[drupal-devel] [feature] common.inc Move textarea pre hook between title and textarea
by Bèr Kessels 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/18828 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jbond Updated by: Bèr Kessels Status: patch +1 From me. Any module that adds something, will want to add it after the title. I really cannot think of any case where aditional content should sit above the title, but I can think of loads of cases where content needs to sit between the title and textarea. Bèr Kessels Previous comments: ------------------------------------------------------------------------ March 13, 2005 - 10:52 : jbond Attachment: http://drupal.org/files/issues/common.inc_10.patch (1.8 KB) This patch moves the textarea hook for 'pre' between the title and the textarea. This makes it easier for simply textarea enhancements like quicktags to add a toolbar just above the textarea. It doesn't appear to have any effect on the fckeditor or htmlarea modules. It doesn't introduce any new hooks. ------------------------------------------------------------------------ March 13, 2005 - 18:56 : killes(a)www.drop.org Guess that is a patch.

1 0

[drupal-devel] [bug] Menu cache is not locale aware
by Bèr Kessels 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/13503 Project: Drupal Version: cvs Component: menu system Category: bug reports Priority: normal Assigned to: chx Reported by: chx Updated by: Bèr Kessels Status: patch Josés latest comment tels in much less wording what i tried to explain too. Still +1 from me, since with i18n (and other possible language modules) you cannot simply flush the cache. This patch is simple, clean and only adds one row per language to your cache table. That cannot be a problem, now, can it? Bèr Kessels Previous comments: ------------------------------------------------------------------------ November 26, 2004 - 08:13 : chx Attachment: http://drupal.org/files/issues/menu_locale.patch (522 bytes) If you have a multi language site, and change languages, the menu is already cached, thus it can not display the translated version. This simple patch makes the menu cache locale aware. Do I need to file the same patch against HEAD? 'Cos it applies, too. ------------------------------------------------------------------------ November 26, 2004 - 10:13 : Dries Isn't it better to flush the menu cache when someone (or the admin) switches languages? Keeps the cache size down. ------------------------------------------------------------------------ November 26, 2004 - 12:20 : chx There could be several points where the language is changed. i18n for example displays a language selection block which lets you switch anytime. Thus I think this solution is the best. ------------------------------------------------------------------------ November 26, 2004 - 13:26 : Goba See also this issue [1], which is still (again) open. The locale is also changed, when the admin changes it as the default on the admin interface, and the menu cache needs to be flushed then, and the admin needs to be redirected. I don't know how these problems are solved currently, so I would only be able to submit a placebo patch to make that issue pop up again... [1] http://drupal.org/node/11312 ------------------------------------------------------------------------ November 26, 2004 - 14:28 : Bèr Kessels I solved it by adding some logic to the swithcer function in i18n: when a language is switched the cache is flushed. The negative side of this, is that if you have hundred various roles of users reading English and one switching to German, the complete cahce is flushed. But then again, who has so many roles, that this might become an issue? ------------------------------------------------------------------------ November 27, 2004 - 10:59 : Dries I just fixed bug 11312 [2]. I don't see what's wrong with invalidating the user's menu cache when he or she switches language. Just do: <?php cache_clear_all("menu:$user->uid"); ?> [2] http://drupal.org/node/11312 ------------------------------------------------------------------------ January 30, 2005 - 20:26 : Jose A Reyero This is needed also for i18n. Invalidating cache is not a solution here, as we would need to do it also for anonymous users. So I think this is a simple solution and will work fine for both cases, user/locale and i18n. ------------------------------------------------------------------------ January 31, 2005 - 16:15 : Bèr Kessels I found out what is wrong with this: Swithcing and cache are two completely differetn things: The language is a per-session setting. The cache is a per-role setting. So any user can switch langauge on any moment, resulting in a cahce that will get flushed on nearly every page-load. And since roles and sessions are tow different thingsm we should not flush a role based cache because a variable in the session changes. The only correct option would be to have per-locale cache. (as per Jose's patch) ------------------------------------------------------------------------ January 31, 2005 - 16:27 : Bèr Kessels Correction, It was not José that submitteed the patch for locale aware menu's but Károly Négyesi, I got confused because José was the last to comment on that patch. ------------------------------------------------------------------------ February 2, 2005 - 21:44 : moshe weitzman i can't make heads or tails of this patch request. it seems that some folks are confusing menu cache and page cache. and we don't even have a role cache. dries' question remains unanaswered. why can we not just rebuild a user's menu cache when needed? ------------------------------------------------------------------------ February 2, 2005 - 22:17 : Jose A Reyero /why can we not just rebuild a user's menu cache when needed?/ This way you cannot have menus in multiples languages for anonymous users, unles they're not cached at all. ------------------------------------------------------------------------ February 2, 2005 - 22:27 : moshe weitzman Aha! Makes sense now. +1 for this simple patch. ------------------------------------------------------------------------ March 13, 2005 - 23:10 : killes(a)www.drop.org I agree, this should get patched. Still applies. ------------------------------------------------------------------------ March 14, 2005 - 00:24 : stefan nagtegaal Tested against 4.6 RC I and works perfectly! +1 for applying this bug-fix..

1 0

[drupal-devel] Reviewing bugs and patches
by Gerhard Killesreiter 14 Mar '05

14 Mar '05

Hi there, I've just spent several hours of reviewing/testing patches and bug reports on drupal.org. As a result, there are now less than three pages of patches here: http://drupal.org/project/issues?projects=3060&states=8 and 14 pages of bug reports here: http://drupal.org/project/issues?projects=3060&states=1&categories=bug (I am not going to look at the feature requests...) Among those bug reports are probably still a lot of duplicates and issues which no longer apply. It would be nice if everybody on this list (at least the regular contributors) could maybe review five bugs/ patches per day. We could also get together on IRC at some time and have a bug fixing session together. Dries and Steven could wait in the background and apply the resulting patches as they come... Cheers, Gerhard

3 2

[drupal-devel] [feature] introducing theme_term_link()
by Anonymous 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/12154 Project: Drupal Version: 4.5.0 Component: taxonomy.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Bèr Kessels Updated by: Anonymous Status: patch What did the usability meeting say about this? Nothing. Has not been discussed there. I personally still think that *all* HTML should pass through a theme function, and so should this listing. And no, passing it trough one of the two listing theme functions is not a good idea. Because this is not suc a list. This is the list of terms that show up with a node. Not a list that shows up in blocks or boxes! Its useage is completely different from these listings. Bèr Anonymous Previous comments: ------------------------------------------------------------------------ October 26, 2004 - 20:40 : Bèr Kessels Attachment: http://drupal.org/files/issues/drupal_84 (1.28 KB) There currently is *no* way to theme a term link. Thats not good [tm]. Because I think this is not just a simple featurem, but required for good themeing (note that even "silly-er" functions have extremely complex theme functions) I categorise it as a Bug and not a task or a feature! This patch allows themes to override the term-links by themeing a very simple function. ------------------------------------------------------------------------ October 26, 2004 - 22:02 : moshe weitzman why would someone want to theme a term link? how is this different from links to users or links to nodes. ------------------------------------------------------------------------ October 26, 2004 - 22:25 : Bèr Kessels I want it for one: <?php function remixreading_term_link($term) { return taxonomy_image_display($term->tid)." ".l($term->name, 'taxonomy/term/'. $term->tid); } ?> and other might want fancy opportunities here: <?php function mytheme_term_link($term) { if(in_array($term->vid, $array_of_fancy_vocs)) { return '<div id=voc-'.$term->vid.'">'.l($term->name, 'taxonomy/term/'. $term->tid).'</div>; } else { return l($term->name, 'taxonomy/term/'. $term->tid); } } ?> so Yes!, i believe its areal useable issue here! ------------------------------------------------------------------------ October 27, 2004 - 00:12 : drumm Setting status to patch. +1 for what is here and another request: The image and forum modules should be able to make the links go to the image and forum modules. Although this could easily fit into http://drupal.org/node/10425. ------------------------------------------------------------------------ October 27, 2004 - 05:50 : Eric Scouten "The image and forum modules should be able to make the links go to the image and forum modules. " I strongly agree here. Related suggestion: I'd like it if taxonomy_menu could "hijack" the term listings so that clicking on a term caused the menus to update properly. (IOW, the term link should be to taxonomy_menu/(vid)/(tid), not to taxonomy/(tid). This may or may not fit with the patch provided here. (I didn't read the patch.) ------------------------------------------------------------------------ October 27, 2004 - 12:28 : moshe weitzman i guess folks want image and forum links to be rewritten because thos modules output their nodes using a display that differs from node/x. that is image/x looks different from node/x. the right way to fix that is to do what we did for book.module. unify the look of image/x and node/x such that we no longer have an imagex. see how book.module accomplishes this. i am still -1 for this patch since i see many other types of links that could be themed in this way (users, forum nodes, recipe nodes, polls, etc.). on the other hand, it is a pretty small change. ------------------------------------------------------------------------ October 27, 2004 - 12:30 : moshe weitzman i guess folks want image and forum links to be rewritten because thos modules output their nodes using a display that differs from node/x. that is image/x looks different from node/x. the right way to fix that is to do what we did for book.module. unify the look of image/x and node/x such that we no longer have an imagex. see how book.module accomplishes this. i am still -1 for this patch since i see many other types of links that could be themed in this way (users, forum nodes, recipe nodes, polls, etc.). on the other hand, it is a pretty small change. ------------------------------------------------------------------------ October 27, 2004 - 17:39 : Eric Scouten No, false assumption. There is no image/x URL for displaying a single image. The issue is not how to display a *single* node (we all agree that node/x is correct), but how to display a *list* of nodes. For certain kinds of nodes, there are far better ways to do it than offered by taxonomy_render_nodes, so it makes sense to allow an override of that list display. (Note: This is straying from the original discussion. If I didn't have to leave ASAP, I'd start a new thread.) ------------------------------------------------------------------------ November 15, 2004 - 10:47 : Bèr Kessels bringing this back to the original discussion: pass the taxonomy links trough a theme function, so that people can, for example, add icons to the links. The patch sill applies (hint,hint!) ------------------------------------------------------------------------ November 15, 2004 - 15:24 : moshe weitzman Perhaps we do this themeing not for every taxo link but rather for lists of links. In other words, do like theme_node_list() and theme_user_list(). more consistent with existing practice. ------------------------------------------------------------------------ March 13, 2005 - 20:49 : killes(a)www.drop.org What did the usability meeting say about this?

1 0

[drupal-devel] [feature] introduce a "cron" user to fix various cron related issues
by Anonymous 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/5380 Project: Drupal Version: cvs Component: user system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: ax Updated by: Anonymous Status: patch Hi, The functionality is great, but your interface is not. IMO you should just choose good defaults and leave it with that. Why would users want to choose "UIDs" ofr "cron Jobs". Joe user should not have to think about UIDs and Crons. IT should Just Work[tm]. I know there are lots of places in Drupal where this is not the case either, but we are working on that. So when we introduce new functionality we should do it good at once. Why not go for Stefans suggestion of choosing a cron UID and jsut leave it like that. UIDs are not really used anyway, so you can choose just any UID (just the next one available, most often will be '2'). Bèr Anonymous Previous comments: ------------------------------------------------------------------------ January 23, 2004 - 23:33 : ax we currently have some problems caused by the undefined state of $user in the cause of cron runs. sometimes it's "Anonymous", sometimes it picks up the currently logged in user (i think). I stumbled upon this when reading issues closed by "Anonymous" caused by the automatic cron closure when issues are fixed. i always think "isn't it strange that "Anonymous" can close issues just as she feels like?" there are quite some related issues - search this site for "cron user". couldn't a cron user solve these problems? a la if (REQUEST_URI) == cron.php then $user = <cron_user>, with <cron_user> having some special properties such as showing "Automatically" for closed issues, being possible to excluded in statistics, putting the rss author from imported rss nodes into nodes author field, having special access checks etc. i didn't think about this much, but thought it is worth filing here. please discuss, elaborate - and don't flame me if i'm too off ;) ------------------------------------------------------------------------ February 11, 2004 - 05:04 : Chris Johnson Sounds like a good idea to me. It's one of many things I'd like to change if I get time to work on it. ------------------------------------------------------------------------ September 27, 2004 - 19:25 : Philippe Couln't we just write this in cron.php: <?php $cronuser = variable_get('cron_cronuser','nobody'); if ($cronuser = 'nobody'){ $user = user_load(array()); //I believe this results in Anonymous?? }else{ $user = user_load(array('name' => $cronuser)); } ?> We can set the variable cron_cronuser in a page like admin/cron. If the site administrator doesn't set a value, cron uses Anonymous for everything (not some user that happens to be logged on). If a valid user name is set, the site admin can give the cron user special privileges. ------------------------------------------------------------------------ October 1, 2004 - 18:41 : Philippe Setting the user like this in cron.php sends a cookie. (How? Where? Can we prevent this?) So everybody can get "cron" privileges by loading cron.php once. We could fix this by making cron.php accessible only from localhost (in .htaccess). Any ideas? ------------------------------------------------------------------------ October 1, 2004 - 21:11 : Philippe Attachment: http://drupal.org/files/issues/cron_uid.patch (3.62 KB) This patch lets the admin select a UID for cron in the admin/settings page. The variable name in which this is stored is 'cron_uid'. When cron.php is loaded, it sets a variable $cron_is_running. If this variable is set, no session will be opened (and no cookies will be sent). The cron page will use the Anonymous user (uid==0) if cron_uid is not set. Note that the site admin should create a "cron" user before setting the UID. ------------------------------------------------------------------------ March 13, 2005 - 19:18 : Chris Johnson Attachment: http://drupal.org/files/issues/cron_uid2.patch (3.19 KB) Previous patch still applies cleanly to HEAD and code algorith looks good on inspection. However, there are some style problems and one ugly bit using StdClass(), so I have re-rolled the patch. ------------------------------------------------------------------------ March 13, 2005 - 19:37 : killes(a)www.drop.org I like the idea, some comments, though: +if (!$cron_is_running) { + session_start(); +} Is this neccessary? 'User ID '.$cron_uid.' does not exist' ------------------------------------------------------------------------ March 13, 2005 - 20:11 : killes(a)www.drop.org 'User ID '.$cron_uid.' does not exist' < == needs to be made translatable. Setting to active. ------------------------------------------------------------------------ March 14, 2005 - 05:56 : Chris Johnson Attachment: http://drupal.org/files/issues/cron_uid3.patch (3.29 KB) Ooops. Sorry I missed that text that needed translating. New patch attached with that bit corrected. Yes, I think the if (!$cron_is_running) code is needed to keep the cron user from creating many unneeded sessions. ------------------------------------------------------------------------ March 14, 2005 - 09:48 : stefan nagtegaal I am all for the idea of introducing a special id for running the cronjobs, but the approach you took here is far from perfect imo.. After looking at your patch, there are several questions and remarks that came up to my mind: I agree with Gerhard that the $cron_is_running is probably unneeded, I think it's better todo: <?php if (!variable_get('cron_busy', true)) { session_start(); } ?> I think that this is much to technical. Severa newbies will probable ask what a user id is, or what cron jobs are.. From both usability and userfriendlyness this patch is a big -1 from me.. I would suggest todo the following: - Make a new user in the db called 'Cron' with a user id -1, or something like that.. That way you don't have to add extra options to the administration, and the way of implementing should be much cleaner. Also does it improves usability... What do you think?

1 0

[drupal-devel] Filtering comment subjects
by Tim Altman 14 Mar '05

14 Mar '05

Greetings, I'm using bbcode.module on a site and I'm seeing comment subjects with raw bbcode in them. I checked comment.module and subjects aren't run through input filters, so any input filter could end up in comment subjects. Is there a function that can strip filters that could be applied to comment subjects when they're created? -- Tim Altman

2 1

[drupal-devel] Project module
by Claus Rasmussen 14 Mar '05

14 Mar '05

Hi folks, I'm trying to run the project.module on 4.6RC with PHP5 and MySQL 4.1. In the function project_page_user a query gets rewritten to: SELECT DISTINCT(n.nid), n.title, n.changed, COUNT(p.DISTINCT(n.nid)) AS count FROM node n LEFT JOIN project_issues p ON n.nid = p.pid AND p.state IN (1,2,8) WHERE n.type = 'project_project' AND n.status = 1 AND n.uid = 1 GROUP BY n.nid ORDER BY n.title ASC But it seems that the result should really be the following? (difference in the COUNT DISTINCT): SELECT DISTINCT(n.nid), n.title, n.changed, COUNT(DISTINCT(p.nid)) AS count FROM node n LEFT JOIN project_issues p ON n.nid = p.pid AND p.state IN (1,2,8) WHERE n.type = 'project_project' AND n.status = 1 AND n.uid = 1 GROUP BY n.nid ORDER BY n.title ASC The module still doesn't quite work (php error log: PHP Fatal error: Cannot use object of type stdClass as array in /path/to/modules/node.module on line 380, referer: URL/project) but before looking into that, isn't the query rewrite wrong? /claus

3 2

[drupal-devel] [bug] Do not display poll block when no polls are active
by Prometheus6 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/14767 Project: Drupal Version: cvs Component: poll.module Category: bug reports -Priority: minor +Priority: normal Assigned to: Prometheus6 Reported by: Prometheus6 Updated by: Prometheus6 -Status: active +Status: patch Attachment: http://drupal.org/files/issues/POLL_HEAD_PATCH.txt (968 bytes) Prometheus6 Previous comments: ------------------------------------------------------------------------ December 25, 2004 - 06:40 : Prometheus6 Attachment: http://drupal.org/files/issues/poll.patch (1.14 KB) I use polls infrequently and want them on the sidebar when they are active. But I don't want an inactive poll displayed for weeks or months. This patch changes the SQL that selects the latest poll to display. It simply adds the requirement that the poll be active. ------------------------------------------------------------------------ December 25, 2004 - 06:41 : Prometheus6 Should be vs. CVS instead of 4.5 ------------------------------------------------------------------------ December 26, 2004 - 15:02 : Dries "JOIN poll p": should be "JOIN {poll} p" or things will break when database prefixing is used. "on p.nid=n.nid": should be "ON p.nid = n.nid". Use capital letters for SQL keywords, and put spaces around "=". Other than that, this looks like a bug fix. ------------------------------------------------------------------------ December 26, 2004 - 17:28 : Prometheus6 Attachment: http://drupal.org/files/issues/poll_0.patch (1.19 KB) Bug fix, yes, but I figured if no one complained in so long it was intentional. Or maybe most people that use polls use them frequently. Two corrections to formatting, and braces around the table name added. ------------------------------------------------------------------------ March 13, 2005 - 15:16 : killes(a)www.drop.org patch: **** malformed patch at line 12:

1 0

[drupal-devel] [bug] Error with file api and open_basedir restriction
by Jose A Reyero 14 Mar '05

14 Mar '05

Issue status update for http://drupal.org/node/5961 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal -Assigned to: Jose A Reyero +Assigned to: Anonymous Reported by: Jose A Reyero Updated by: Jose A Reyero -Status: active +Status: patch Attachment: http://drupal.org/files/issues/file-open-basedir_02.patch (810 bytes) Just resubmitted the patch. The problem is, from the time I sent the first patch , I've moved all my sites, no shared webhosting anymore, so I don't have that upload trouble anymore, and also cannot test it myself. That's also why I'm unassigning this issue to me. So please, somebody else test it. Jose A Reyero Previous comments: ------------------------------------------------------------------------ February 20, 2004 - 20:45 : Jose A Reyero Attachment: http://drupal.org/files/issues/file-open-basedir.patch (1018 bytes) I get this error when uploading a file through the file api, and the upload fails: warning: open_basedir restriction in effect. File is in wrong directory in... This happens when open_basedir restriction is activated and the temporary directory is not in the allowed directories. There's a related bug in the profile.module: http://drupal.org/node/view/2648 [1] FIX: The solution is to upload files using php's 'move_uploaded_file' -which works even when open_basedir restriction is in effect. This patch checks for the error conditions in 'file.inc::file_check_upload', and then uses move_uploaded_file to upload the file to the temporary folder with a random name. Otherwise -if the error conditions are not met-, does nothing. RESTRICTIONS: This patch introduces one additional restriction for the file api: Once you have called 'file_check_upload', you have to use the returned object when calling later 'file.api::file_save_upload'. This shouldn't be a problem, as the right way to upload files should be -I think, please correct me if I'm wrong-: - $file=file_check_upload(); - (Perform validations with $file) - file_save_upload($file,.....) I will post also a very simple patch for profile.module to work with this one. Anyway, as mentioned above, if the error conditions are not met, the patch does nothing, so I think it should be applied first and then check for other modules (profile.module) to use the file api the right way and then be able to upload files when open_basedir restriction in effect. [1] http://drupal.org/node/view/2648 ------------------------------------------------------------------------ November 28, 2004 - 18:38 : Goba This seems to be an interesting alternate fix to my dump patch for locale.module [2], which is the very same issue... Moving the file out of the upload folder right away to the Drupal temp folder seems to be a good idea to me. The only caveat is that files in the upload folder get removed automatically, while one needs to remove the file from the Drupal temp folder, once it is not needed anymore! [2] http://drupal.org/node/13285 ------------------------------------------------------------------------ December 8, 2004 - 20:26 : Goba Please look at this stuff. We need to solve open_basedir problems at least in core before the next point release. There are quite a few users trying to use Drupal on shared hosting here, where open_basedir is in effect. ------------------------------------------------------------------------ February 18, 2005 - 02:55 : clydefrog This functionality gets a +1 from me, the user. We solved this problem by changing upload_tmp_dir in the server config, but a fix in Drupal would be more elegant. ------------------------------------------------------------------------ March 13, 2005 - 20:41 : killes(a)www.drop.org Doesn't apply anymore.

1 0