CRM-18799 - remove use of exec in WordPress bootstrap

[MegaphoneJon]

• CRM-18799: different server requirements between 4.7.x and 4.6.x

[kcristiano]

Tested on WordPress 4.7.2 and current civicrm-master. Patch works as expected. I was able to reproduce error in CRM-18799 by disabling 'exec'. I then applied the patch and upload and browsing of files in CK editor worked.

[totten]

Thanks for testing @kcristiano !

Random thoughts @PalanteJon @kcristiano :

• Since this is basically the same file as https://github.com/civicrm/cv/blob/master/src/Bootstrap.php , a similar patch probably belongs there.
• I think the purpose of getSearchDir() is to figure out the absolute path of the search dir (so it can feed into in findCmsRoot() which walks up from this path to the root of the filesystem). In my quick testing on CLI, SCRIPT_FILENAME could return a relative or absolute path (depending on how the command was called). (In fact, in the cv use-case, the SCRIPT_FILENAME will never be pointing where you want because cv is in some bin dir.) Possible compromise: skip the SCRIPT_FILENAME check if PHP_SAPI === 'cli'?
• The original goal in using exec('pwd') was to resolve the PWD in a way that preserves symlinks (i.e. not realpath()). Looks like getenv(PWD) does that. :)

[kcristiano]

@totten good points. I will test on a host with symlinks that have driven me crazy in the past.

As far as patching 'cv' I do think that is a good idea. Realistically any host that allows cli access will probably have exec privileges. your compromise sounds good to me.

[kcristiano]

@totten took a stab at this over here: kcristiano/cv@0838d41 full source https://github.com/kcristiano/cv/blob/master/src/Bootstrap.php

I have made the corresponding change on wp-master (buildkit) and have no issues so far. Once I have some feedback, I can push this out to a site where I can trigger external scripts (mailing urls, paypal ipn etc) for further testing.

[eileenmcnaughton]

@kcristiano what do you think the status is here. It's a wordpress only patch so I will defer to your expertise on whether to merge this

[kcristiano]

@eileenmcnaughton My only concern is the related patch at kcristiano/cv@0838d41

I've tested @PalanteJon 's patch on a number of WP environments and it is an improvement I would like to see. I have not had time to test when a extern scripts get called. I would want to do that before merging. The changes Tim had made in both cv and civicrm.config.wordpress make the calls to extern more reliable and I want to be sure we do not have a regression.

[kcristiano]

@eileenmcnaughton I've (finally) gotten back to this. I have tested with extern calls as well:

• PayPal Standard Payment Processor -- IPN is received and recorded properly
• extern urls in mailings are not returning the correct path consistantly

I'll take a look at why this has happened

@totten feedback would be appreciated on the relevent changes to cv

[agileware]

Quick note here - internally unless -P is specified or the POSIXLY_CORRECT environment variable is set, the GNU coreutils pwd program will effectively returns getenv("PWD") and a newline unless something is seriously warped with it.
@kcristiano With that in mind, do you see extern urls behaving correctly without the SCRIPT_FILENAME condition?
Given the prevalence of the exec call being disabled in shared hosting environments we really need to merge a version of this that doesn't depend on it.

[kcristiano]

Thanks @agileware I did test without CRIPT_FILENAME and the url was just /civicrm/extern/... and that gives a 404, so we do need further work on this.

[agileware]

@kcristiano are you sure that's related? I can't see where the mail URL generation would depend on this code.

[kcristiano]

@agileware It's the handling of the extern/ links in the email. Without the patch the url is generated properly, with it the url adds the full path. We had this issue before with how the URLs are handled differently for WP so it's part of what I test.

[kcristiano]

@agileware @PalanteJon Tested against current master 4.7.20

• Applied Patch
• Created email with external links
• sent email via SMTP
• email arrived with proper links
• links worked
• created donation form
• set up PayPal as processor
• made donation
• PayPal sent IPN, contribution record updated properly

Patch works as it should, unsure why I had earlier failure.

reproduced this in a second server and achieved the same results - success.

@totten OK to merge.

[eileenmcnaughton]

Merging based on @kcristiano go ahead