Provide find_conf_path() function by re-factoring reusable code from existing conf_path() function.

(EDITED to reflect the status of the latest patch submission)

Re-factor the conf_path() function to create find_conf_path(), url_to_path(), and url_to_realpath() functions.

/**
 * Find the appropriate configuration directory for a given host and path.
 *
 * @param $http_host
 *   The hostname and optional port number, e.g. "www.example.com" or
 *   "www.example.com:8080".
 *
 * @param $script_name
 *   The part of the url following the hostname, including the leading slash.
 *
 * @return
 *   The path of the matching configuration directory.
 *
 * @see conf_path()
 */
function find_conf_path($http_host, $script_name, $require_settings = TRUE);

/**
 * Returns the local relative path corresponding to a given URL, if possible.
 *
 * Finds the configuration directory matching the given URL, and compares it
 * to the current configuration directory. Returns a path relative to the drupal
 * install if they match, and FALSE if they don't.
 *
 * In a properly-configured multisite installation, this function helps answer
 * the question, "Could a given URL match a file or path within my site?"
 *
 * Whether the URL does in fact resolve to this site, or at all, cannot be
 * determined within Drupal itself.  This is only a sanity check.
 *
 * @param $url
 *   The internal path or external URL being linked to, such as
 *   "drupal/node/34" or "http://example.com/drupal/foo".
 * @return
 *   FALSE if $url contains a host/port/path that does not match the current
 *   site, or else a drupal path such as "node/34" or "foo".
 *
 * @see conf_path(), find_conf_path()
 */
function url_to_path($url);

/**
 * Returns an absolute local path corresponding to a given URL, if possible.
 *
 * For example, when looking for image URLs within an email message body for
 * possible conversion to inline attachments, the following code might be used:
 * @code
 *   if ( !url_is_external($url)
 *     && ($path = url_to_realpath($url))
 *     && file_exists($path) ) {
 *     // Attach $path and replace $url.
 *     ...
 *   }
 * @endcode
 *
 * @param $url
 *   The internal path or external URL being linked to, such as "node/34" or
 *   "http://example.com/drupal/foo".
 * @return
 *   FALSE if $url contains a host/port/path that does not match the current
 *   site, or else an absolute local path such as "/path/to/drupal/node/34".
 *
 * @see url_to_path()
 */
function url_to_realpath($url);

@Damien -- Of course I know this is 8.x but the patch applies cleanly to both and I wanted a testbot check.

Revised patch fixes a couple of whitespace errors.

Can we please leave at 7.x until the testbot returns something?

Another typo, sigh.

Cool. Testbot passed. Once we have a working 8.x testbot, I'll re-roll and re-submit, but for now the code is identical.

Suggestions taken.

P.S. -- Joachim: Please help me understand your "ew" response. Standards? Coding style? Documentation? -- Thanks.

P.P.S. --The function should probably be named url_to_path() since it takes a (possibly absolute) URL and returns a local path, or FALSE if no equivalent local path could exist.

Go, testbot!

Use issue summary template.

Suggestions implemented and name changed.

Found and fixed a bug, and wrapped the return value in drupal_realpath() for good measure.

Better doxygen comments.

The returned value should be directly usable by fopen(), file_exists(), and the like.

Perhaps this should use realpath() instead of drupal_realpath()?

Perhaps a better name would be url_to_local_path()?

Did some testing and found I was using parse_url() incorrectly.

Replaced drupal_realpath() with realpath() because I can't imagine a need for stream wrappers with this function.

@#20 -- Almost. Actually, for most paths, url_to_path(url($path)) == realpath('./' . $path)

So maybe this function should be called url_to_realpath()?

Do we have a working D8 testbot now?

Alternatively, why not supply both url_to_path() and url_to_realpath() functions?

@Damien -- I would expect that realpath($path) == DRUPAL_ROOT . '/' . $path for most relative paths, but calling realpath() has the advantage of resolving symlinks to a canonical path.

Attached patch changes substr($path, 1) to ltrim($path, '/') because $path may or may not start with a leading slash.

Here's the goal. If Drupal is installed at http://example.com/drupal and conf_path() is 'drupal/sites/example.com' then:

1. url_to_path('http://external.com/drupal/foo') returns FALSE
2. url_to_path('http://www.example.com/drupal/foo') returns 'drupal/foo'
3. url_to_path('http://example.com/foo') returns 'foo'
4. url_to_path('drupal/foo') returns 'drupal/foo'
5. url_to_path('/foo') returns 'foo'

#3 and #5 should perhaps return FALSE, but that would require logic not provided by the existing conf_path().

@Damien -- You're right, it only adds one line of code to take base_path() into account.

Revised patch so that if conf_path() is 'drupal/sites/example.com' as before,

1. url_to_path('http://external.com/drupal/foo') returns FALSE
2. url_to_path('http://www.example.com/drupal/foo') returns 'drupal/foo'
3. url_to_path('http://example.com/foo') returns FALSE
4. url_to_path('drupal/foo') returns 'drupal/foo'
5. url_to_path('/foo') returns FALSE
6. url_to_path('/drupal/foo') returns 'drupal/foo'

I really need to write some tests.

As an aside,

function conf_path($require_settings = TRUE, $reset = FALSE) {
  $conf = &drupal_static(__FUNCTION__, '');

Why drupal_static() rather than plain static? Surely the value can't change within a single page request, can it?

According to rfay it isn't (currently) possible to test this on Drupal testbots, so removing "needs tests" tag.

See #1117308: It should be possible to test things that depend on base_path() and conf_path()

Found and fixed another minor logic error.

Updated doxygen to acknowledge that realpath( string $path ) does a stat and returns FALSE if the $path does not exist. Also moved url_to_realpath() after url_to_path(). No other changes.

@catch -- No profiling is necessary. The only core change is that part of conf_path() moves to a new function called find_conf_path(). If it kills performance to add just one function call, then I need to switch programming languages.

Tagging as per http://drupal.org/node/1050616#comment-4327740

@catch -- huh? What are you looking at?

EDIT: -- Okay, I can see where, looking at the issue description and (only) the top of the patch, you'd get confused.

The patch modifies both bootstrap.inc and common.inc.

In bootstrap.inc, most of conf_path() is moved to a new function, find_conf_path(). The patched conf_path() calls find_conf_path().

In common.inc, two new functions are added: url_to_path() and url_to_realpath().

When writing url_to_path(), I needed part of the conf_path() function. I didn't want to copy part of conf_path() into url_to_path(), so I used that part to make a new find_conf_path(), instead.

Better?

See related issue #1160374: emogrifier path issue when base_path() and DRUPAL_ROOT contain same 'sitename' folder

It turns out that I can't use DRUPAL_ROOT in conjunction with base_path().

Updated patch for 8.x:

Note that the code in this issue has been used by Emogrifier since April 5, 2011:

• 6.x-1.7
• 7.x-1.7
• 8.x-1.12

That's 172 users, according to the project usage statistics.

Does that qualify as "Reviewed and Tested by the Community" ?

Apparently not, (sigh). Tagging instead.

#38: url_to_path-1113588-38.patch queued for re-testing.

#38: url_to_path-1113588-38.patch queued for re-testing.

The code style needs to be fixed everywhere (docblocks are not correctly formatted, ternary operators should not go to the line, etc.)

I'll review and try to fix (I've learned some things since writing that code), but if I fail to see the same things you're seeing, I'd appreciate some specific examples.

url_to_path() is buggy as it uses $base_path which is the base path *of the current site* (ie. it is a part of $base_url). The base path could be different for other sites

Well, that's exactly what it's supposed to do. The object is to answer the question, "Could this url resolve to a file on the current site?" I'm not concerned with other sites that coincidentally share the same server.

- url_to_realpath() has very limited added value and should just go away

Actually, that's the only function that is necessary for my use-case.

Quoting my own post:

The relevant code snippet is found in mailmime.inc:

protected function attachRegex($matches) {
    if ( ($url = filter_xss_bad_protocol($matches[4], FALSE))
      && ($path = url_to_realpath($url))
      && is_file($path)
      && $this->addHTMLImage($path)
    ) {
      // The parent method will replace this with the actual cid: string.
      $matches[4] = $path;
    }
    return implode('', array_slice($matches, 1));
  }n

So if:

• The link address passes the cross-site-scripting filter, and
• The url can be resolved to a local path, and
• There is a file at that path,

then the file is attached.

Okay, I've tried to address your concerns, but I don't understand what is wrong about the formatting of my doc blocks. Can you point me to a specific example that violates coding standards?

In other circumstances the developer would often be much better off refactoring their code to avoid this kludge.

I'm sorry that you find my solution inelegant. Can you suggest an alternative way for me to safely resolve a URL to a file within the current site? Note that unlike the solution @sun provides in his Local image input filter, I want to include the possibility that several domains point to the same Drupal site.

It is hard for me to imagine a case where copying 90% of a core function into module code just to avoid running the other 10% would be preferable to refactoring that core function into reusable parts.

Nevertheless, I am maintaining two such copies, both in Emogrifier and in Mail MIME.

I suppose that I could write an entire module just to supply the missing function, but that seems ... ugly.

Acknowledge bfroehle's redaction and add another possible use-case.

Corrected awkward grammar.

Well, given that limited use case and the comments above, I recommend only getting in the bootstrap.inc part.

Fair enough. Re-rolled. (Thanks, Lars!)

Note that it would be nice to consider other sites that coincidentally share the same server, but doing so with a reasonable level of confidence would require (at least) a DNS lookup starting from the global root servers, and possibly some assumptions regarding the webserver and filesystem configuration.

Updated title and summary.

Linked to bfroehle's redacted comment.

Updated to reflect RTBC status.

Thanks. Added an API change node (please review -- I don't know what belongs in most of the fields), updated the summary, and removed the "Needs committer feedback" tag.

Nonetheless, it is too bad that there isn't another more robust way to accomplish the same thing --- like a custom theme_image which would alter the URL and record that the image should be attached.

Are you referrring to this function?

@jessebeach:
Please open a new issue and link it here. Also please verify whether reversing the commit patch solves your problem, as I do not see any possible way that your problem could be related to the change made in this issue.

Note that none of the code lines referenced by your screenshots have been changed; they were just moved without modification from one place to another.

EDIT: Retracted -- see next comment, and thanks for the report!

Bah! Now I see the problem. The parameters $script_name and $http_host somehow got reversed. (Scrambling to find/fix collateral breakage...)

Do we not have any multisite tests?

No.

And if not, how the heck would we write any?

Dunno. I don't see any open issues. Feel free to open one.

EDIT: Nevermind. Opened #1294974: PIFR should provide a way to test multisite installations.

At least one of the following should be closed as a duplicate:
#1117308: It should be possible to test things that depend on base_path() and conf_path()
#1294974: PIFR should provide a way to test multisite installations.
#1295046: Determine testing status of multisite installs (and if it is possible)