The function mailsystem_create_class() is working OK unless the files directory of a site is physically stored outside of the Drupal root. This is the case e.g. if you have Drupal installed in /var/www and the /var/www/sites/default/files is a symbolic link to e.g. /mnt/files

Some hosting strategies are locating the writable directories outside of the Drupal root for security reasons and/or for better permission management and/or for an easier backup strategy.

In this case we end up with the new class in the registry table as '/mnt/files/mailsystem//MimeMailSystem__SmtpMailSystem.mail.inc' or what ever the class name happens to be. But such a class won't be found by Drupal's autoloader as it expects all the classes to be defined relative to the Drupal root.

This module currently tries to achieve this by stripping the Drupal root from the class file name but that fails when that name is outside of the Drupal root.

A pretty simple patch is resolving that issue and of course produces the same result for all the default cases where the files directory is inside of the Drupal root.

I'll submit that patch in a second.

CommentFileSizeAuthor
#6 2756205-6.patch1.6 KBshubham.prakash
#4 interdiff-2756205-3-4.txt553 bytesloziju
#4 mailsystem-fix_for_private_file_outside_drupal_root-2756205-4.patch1.51 KBloziju
#3 mailsystem-fix_for_private_file_outside_drupal_root-2756205-3.patch1.33 KBloziju
#2 class_filename_should-2756205-2.patch1.55 KBjurgenhaas

Issue fork mailsystem-2756205

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jurgenhaas created an issue. See original summary.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Active » Needs review
StatusFileSize
new class_filename_should-2756205-2.patch1.55 KB
loziju’s picture

loziju commented
StatusFileSize
new mailsystem-fix_for_private_file_outside_drupal_root-2756205-3.patch1.33 KB

Patch #2 works. Rerolled the patch for the latest commit in dev branch. Rewrote variable names to follow Drupal coding standards.

loziju’s picture

loziju commented
StatusFileSize
new mailsystem-fix_for_private_file_outside_drupal_root-2756205-4.patch1.51 KB
new interdiff-2756205-3-4.txt553 bytes

The code didn't work well if the private file path is a relative path. This may be due to the different method used in generating the $class_file in the previous dev version.

Attached is the updated patch to check if class file already contains a relative path and thus use that straightaway.

manuel garcia’s picture

manuel garcia commented
Status: Needs review » Needs work 
  1. +++ b/mailsystem.module
@@ -110,6 +110,28 @@ function mailsystem_default_methods() {
+ * @param $to
+ * @return string

    Let's add a description to these. 

  2. +++ b/mailsystem.module
@@ -110,6 +110,28 @@ function mailsystem_default_methods() {
+  if (substr($to, 0, 1) != '/') return $to;

    Coding standards. 

  3. +++ b/mailsystem.module
@@ -149,6 +171,8 @@ function mailsystem_create_class($classes) {
   // Build the class filename.
   $class_file = $class_dir . DIRECTORY_SEPARATOR . "$class_name.mail.inc";
+  // Get relative path.
+  $class_file = mailsystem_get_relative_path($class_file);

    Nit: Let's combine these into one $class_file declaration

shubham.prakash’s picture

shubham.prakash commented
Status: Needs work » Needs review
StatusFileSize
new 2756205-6.patch1.6 KB

Made the changes suggested in #5

solideogloria’s picture

solideogloria commented
Status: Needs review » Reviewed & tested by the community

Been using #6 for a couple months with no problem in combination with this patch: https://www.drupal.org/project/mailsystem/issues/2716371#comment-13328102.

kunal_sahu made their first commit to this issue’s fork.

kunal_sahu opened merge request !4

kunal_sahu’s picture

kunal_sahu commented

Hi I have created an MR , please merge , thanks