Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
base system
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
11 Apr 2007 at 08:16 UTC
Updated:
16 Dec 2007 at 23:40 UTC
Jump to comment: Most recent file
This is a fork from http://drupal.org/node/133789
variable_get('site_mail', ini_get('sendmail_from'));
The above is the present method used when sending e-mail on behalf of the site. The problem with this method is:
- The site_mail variable (the e-mail textfield on the site information page) is not a required field.
- ini_get('sendmail_from') is a Win32 only directive and can be empty.
- An empty FROM field will result in an error.
Possible solutions could include:
- Making the e-mail field a required field.
- Using
$_SERVER['SERVER_ADMIN']as the default, though I believe that this can also be empty. - Using the address
webmaster@domain.comwheredomainis retrieved from the URL. Awebmasteraddress is mandatory for all domains.
Opinions welcome.
-K
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | e-mail-required_0.patch | 1.81 KB | johnalbin |
| #3 | e-mail-required.patch | 936 bytes | Zen |
Comments
Comment #1
wim leers+1
I think the e-mail field should become a required field.
Comment #2
RobRoy commentedYou could still just not visit Site information and this would still be blank even thought it's a "required" field, although I agree that should be done anyways as it would catch most cases.
Comment #3
Zen commentedI agree that making the e-mail field required is an reasonable solution. While the ini_get is basically junk code, I'd much rather have that than a global or a function with a static.
-K
Comment #4
dries commentedI'm ok with this, but it is probably worth documenting. In the form description maybe add something like: "This field is required so your messages will look legitimiate for ...". I dunno, but it feels like we need to indicate why this is important ...
Comment #5
Zen commentedThe description already states
Isn't that good enough?
-K
Comment #6
dries commentedIt's probably enough, but it might be worth being more explicit. It doesn't say why it is so important that this message is _required_. What if I leave it empty? It doesn't tell me about the implications. Not a show-stopping issue but still ...
Comment #7
johnalbinGiven the underlying problem that “an empty FROM field will result in an error,” shouldn’t we also alter
drupal_mail()to prevent empty From fields? Or should we tackle that part of the problem in #133789?Comment #8
johnalbinNew patch adds a slightly more detailed description for
site_mail. And if the user doesn’t update the preferences, it prevents an emptyFromheader indrupal_mail().Comment #9
dries commentedSmall related detail:
modules/drupal/drupal.module: 'mail' => variable_get('site_mail', ''),There is one instance of variable_get('site_mail') that might need an ini_get('sendmail_from').
Might be useful to grep the contributions repository for both mail() (i.e. not using drupal_mail()) and not using ini_get('sendmail_form') with variable_get('site_mail'). That might help to get all of Drupal's mailer activity up to the next level. ;-)
Comment #10
dries commentedFeel free to merge this back with the original patch. I don't mind committing those in one patch. Either works for me. Mark RTBC when ready.
Comment #11
johnalbinPer Dries’ suggestion, I’m merging this patch back with http://drupal.org/node/133789
I’m leaving this issue open until someone (maybe me) gets a chance to search the contributions repository for proper usage of
drupal_mail(). See #9 above.Comment #12
chx commentedWait, the patch is still there and the configuration part is still needed .
Comment #13
jody lynnSite e-mail is now required and asked for during install.