# [PATCH] fix imap_mail actually use the rpath option - Thread: https://externals.io/message/54361 - Original archive: https://news-web.php.net/php.internals/54361 - Started by Keloran on 2011-08-04T15:30:48+00:00 - Messages: 8 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #54361 — Keloran — 2011-08-04T15:30:48+00:00 URL: https://externals.io/message/54361#54361 Index: ext/imap/php_imap.c =================================================================== --- ext/imap/php_imap.c (revision 314217) +++ ext/imap/php_imap.c (working copy) @@ -4016,7 +4016,27 @@ if (!INI_STR("sendmail_path")) { return 0; } - sendmail = popen(INI_STR("sendmail_path"), "w"); + + /** Used to make return-path work **/ + char *sendmail_path = INI_STR("sendmail_path"); + char *appended_sendmail_path = NULL; + + /** Return Path or not **/ + if (rpath && rpath[0]) { + appended_sendmail_path = emalloc( + strlen(sendmail_path) + 3 + strlen(rpath) + 1); + strcpy(appended_sendmail_path, sendmail_path); + strcat(appended_sendmail_path, " -f"); + strcat(appended_sendmail_path, rpath); + sendmail_path = appended_sendmail_path; + } + + /** open the sendmail pointer **/ + sendmail = popen(sendmail_path, "w"); /* New Code */ + + if (appended_sendmail_path) + efree(appended_sendmail_path); + if (sendmail) { if (rpath && rpath[0]) fprintf(sendmail, "From: %s\n", rpath); fprintf(sendmail, "To: %s\n", to); ================ hopefully that will work to fix the rpath option, and finally close this bug http://bugs.php.net/bug.php?id=30688 --- ## #54362 — Pierre Joye — 2011-08-04T15:39:36+00:00 URL: https://externals.io/message/54361#54362 · In reply to: #54361 hi, Can you attach this patch to the bug please? Thanks for your work! > Index: ext/imap/php_imap.c > =================================================================== > --- ext/imap/php_imap.c (revision 314217) > +++ ext/imap/php_imap.c (working copy) > @@ -4016,7 +4016,27 @@ >        if (!INI_STR("sendmail_path")) { >                return 0; >        } > -       sendmail = popen(INI_STR("sendmail_path"), "w"); > + > +       /** Used to make return-path work **/ > +        char *sendmail_path = INI_STR("sendmail_path"); > +        char *appended_sendmail_path = NULL; > + > +        /** Return Path or not **/ > +        if (rpath && rpath[0]) { > +            appended_sendmail_path = emalloc( > +                strlen(sendmail_path) + 3 + strlen(rpath) + 1); > +            strcpy(appended_sendmail_path, sendmail_path); > +            strcat(appended_sendmail_path, " -f"); > +            strcat(appended_sendmail_path, rpath); > +            sendmail_path = appended_sendmail_path; > +        } > + > +        /** open the sendmail pointer **/ > +        sendmail = popen(sendmail_path, "w"); /* New Code */ > + > +       if (appended_sendmail_path) > +            efree(appended_sendmail_path); > + >        if (sendmail) { >                if (rpath && rpath[0]) fprintf(sendmail, "From: %s\n", > rpath); >                fprintf(sendmail, "To: %s\n", to); > > ================ > > hopefully that will work to fix the rpath option, and finally close this > bug http://bugs.php.net/bug.php?id=30688 > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #54364 — Keloran — 2011-08-04T16:07:25+00:00 URL: https://externals.io/message/54361#54364 · In reply to: #54362 patch attached > hi, > > Can you attach this patch to the bug please? > > Thanks for your work! > > > > […] > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #54366 — Pierre Joye — 2011-08-04T16:19:35+00:00 URL: https://externals.io/message/54361#54366 · In reply to: #54364 I have now attached the patch to the issue: https://bugs.php.net/bug.php?id=30688 > patch attached > > > > […] > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #54367 — Keloran — 2011-08-04T16:23:06+00:00 URL: https://externals.io/message/54361#54367 · In reply to: #54366 cool, thanks that patch is for 5.4 (since the line nums changed from 5.3-5.4, > I have now attached the patch to the issue: > https://bugs.php.net/bug.php?id=30688 > > > > […] > wrote: > > […] > this > > […] > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #54370 — johannes@schlueters.de — 2011-08-04T16:48:09+00:00 URL: https://externals.io/message/54361#54370 · In reply to: #54361 Hi, few comments just from reading the patch, not the full context, not testing it: > Index: ext/imap/php_imap.c > =================================================================== > --- ext/imap/php_imap.c (revision 314217) > +++ ext/imap/php_imap.c (working copy) > @@ -4016,7 +4016,27 @@ > if (!INI_STR("sendmail_path")) { > return 0; > } > - sendmail = popen(INI_STR("sendmail_path"), "w"); > + > + /** Used to make return-path work **/ > + char *sendmail_path = INI_STR("sendmail_path"); > + char *appended_sendmail_path = NULL; This won't work on all compiles, declarations have to go first in the block. > + /** Return Path or not **/ > + if (rpath && rpath[0]) { > + appended_sendmail_path = emalloc( > + strlen(sendmail_path) + 3 + strlen(rpath) + 1); > + strcpy(appended_sendmail_path, sendmail_path); > + strcat(appended_sendmail_path, " -f"); > + strcat(appended_sendmail_path, rpath); > + sendmail_path = appended_sendmail_path; > + } > + > + /** open the sendmail pointer **/ > + sendmail = popen(sendmail_path, "w"); /* New Code */ How do you open a pointer ;-) I think everybody reading the code should know `popen()` so the first comment isn't really useful; I assume the second comment there was for your purpose. > + if (appended_sendmail_path) > + efree(appended_sendmail_path); > + Always use { } even for a single line (see CODING_STANDARDS) While editing this functions: Should this function not also respect the new mail.force_extra_parameters and mail.log settings, too? (Might need an extra bug report to be reported correctly ...) johannes --- ## #54375 — Keloran — 2011-08-04T16:56:51+00:00 URL: https://externals.io/message/54361#54375 · In reply to: #54370 Yeah it does need cleaning up, didn't know about the new mail options, main reason I did it was because the function has had the option since 2004, but it's never actually used it > Hi, > > few comments just from reading the patch, not the full context, not > testing it: > > > > […] > > This won't work on all compiles, declarations have to go first in the > block. > > > […] > > How do you open a pointer ;-) > I think everybody reading the code should know `popen()` so the first > comment isn't really useful; I assume the second comment there was for > your purpose. > > > […] > > Always use { } even for a single line (see CODING_STANDARDS) > > While editing this functions: Should this function not also respect the > new mail.force_extra_parameters and mail.log settings, too? (Might need > an extra bug report to be reported correctly ...) > > johannes --- ## #54636 — Keloran — 2011-08-16T15:08:50+00:00 URL: https://externals.io/message/54361#54636 · In reply to: #54375 cleaned up abit more and put in the use of force params, although im not sure using them is the correct thing cant see a way of writing a test for this though unfortunatlly 2011/8/4 Keloran > Yeah it does need cleaning up, didn't know about the new mail options, > main reason I did it was because the function has had the option since > 2004, but it's never actually used it > > On Thursday, August 4, 2011, Johannes Schlüter > wrote: > > […]