Skip root-relative and protocol-relative URLs and support the Pathologic module

Not sure if it is that much faster. Besides if you DO parse the URL, it might be possible to add further domain remapping rules (e.g. identify which are the old source domains to be turned into new CDN'd domains). There are quite a few modules that may generate and store absolute URLs that may need remapping.

Also note that you cannot use valid_url call. I found that in case of filenames that have spaces (e.g. http://absolute.com/files/multi word.png) they could get generated by some image handling modules, pass through browsers, but fail valid_url test. However parse_url handles them right.

Speaking of absolute URL remapping, looks like some image modules generate absolute URL HTML that gets hardcoded into node bodies and teasers and may not even go through your file_create_url. While these issues could be dealt with statically by modules such as "Search and Replace", perhaps CDN module should add a new filter to handle these remaps automagically? The filter would share code with absolute URL checker/mapper in file_create_url.

URL hardcoding becomes a real problem when you have to deal with development vs. production domains

Changing the title to broaden the use case...

Looks like there is another requirement for URL output, in addition to taking care of absolute URLs.

In multi-site environment, files normally go into sub-directories with paths such as sites/MYDOMAIN/files/MYFILE. This makes for very long URLs and for introducing dependence on the domain names into filepath (BAD!). There is a solution by creating /files directory containing a small .htaccess file that would redirect requests made to files/MYFILE (tested and works):

RewriteEngine On
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{HTTP_HOST} ^(www.)?([a-z0-9-.]+)$ [NC]
RewriteRule ^(.*)$ /sites/%2/files/$1 [L]

However, just adding this directory with .htaccess is not enough. All paths generated by file_create_url or stored in nodes need to lose the leading sites/MYDOMAIN fragment.

So, yet another URL post-processing feature is needed. Perhaps it could be added as an optional setting?

P.S. The /files directory solution is described here http://drupal.org/node/53705. In my code above I modified it slightly to remove leading "www.". This was fully tested and works for both WWW and non-WWW cases. There is an additional thread on the issue of removing the leading /sites fragment from the URL: http://drupal.org/node/452308

P.P.S. Looks like such functionality already exists in URL Replace Filter (http://drupal.org/project/url_replace_filter) and Pathologic (http://drupal.org/project/pathologic)? What is the best way to integrate them?

OK, so I had a chance to investigate URL Replace and Pathologic. They fully solve the problem of stripping leading URL segments for both links (href) and image (src) attributes. Works great for any HTML that passes through filters and nothing to add. Pathologic is seen as superior even by other module's author.

These modules however cannot do anything about URLs emitted by CDN module. A great example would be a node with file attachement, that is linked both from file attachement section (through file.inc) and directly linked to absolute path from content. While Pathologic fixes the hardcoded link within content, the attachment section link still comes across as http://www.MYDOMAIN.com/sites/MYDOMAIN/files/MYFILE. That's awful for SEO.

So I modified the CDN core patch to add a section that is run only if Pathologic is enabled. It uses similar (significantly simplified) stripping logic and for its configuration relies on Pathologic setting for default Input Format. I presume the dependency on Pathologic could be completely eliminated if similar setting is added directly to CDN module, but I had no reason to rush this yet. In most cases, the setting is highly likely to be identical.

Would appreciate any comments re: this solution. I am using the code in production now with no problem.

I would describe the current status as "soft production" :)

This is a new multi-site install designed to support a large number of owned-and-operated sites as well as a large number of sites for clients. A few sites are almost released (LOL) but are still kind of under the radar for now... That is why I care so much about fixing site-specific URLs - nothing should look clunky to end users. Not using the daemon just yet but planning to implement soon. First want to stabilize core patches, site functionalities and build scripts.

Greatly appreciate all your work on this module. Very nicely done and hope we could soon make it even better.

As an aside - I just updated the patch to add support for image buttons (form.inc).

Wim, I do not have time to argue whether supporting absolute URLs is "necessary" or "not necessary" and whether using pathologic in core is "acceptable" or "unacceptable".

I am trying to use this module for an actual multi-site setup that has to support real-world requirements. It has to interoperate with many buggy modules and work around users who constantly screw up. From the perspective of actually trying to use your module, it has many shortcomings, which I documented and addressed above. It would be great to incorporate real fixes into the official release. If not, I will of course keep a separate fork for my own use, though this would be quite an unnecessary headache.

Bottom line, I suggest you re-read my description of the problems and if you do not like my solutions suggest your own.

Thank you.

@Wim: Here is an updated patch that runs CDN urls through Pathologic settings. It works great for me, but you may definitely want to review it. All it does is uses Pathologic setting for default filter to process URLs. Note, right now Pathologic module does not provide a nice way to directly call itself from outside, which is why I just did copy/paste of its regexp code. Feel free to update this patch if Pathologic provides better API in the future #758118: wild card for Additional paths

@Dave: I looked at URL Alter module and seems like all it is doing is overriding custom_url_rewrite_inbound() and custom_url_rewrite_outbound()? This is useful, but not sufficient for what CDN module needs. In fact, even before checking out your module I tried to add that same pathologic fix to custom_url_rewrite_outbound(), finding that it does not get called every time it is needed... Is your module supposed to do anything other than override these two functions?

A minor fix to the patch - enforce leading '/' symbol for generated path...

One more little fix to the core patches. When pathologic is used with the latest patch enforcing leading '/', if file_create_url is called on the path several times (for example in Image module) it does not work correctly and adds redundant second '/' character in front.

The solution is to add another condition for file_create_url returning original path - whenever it starts with '/' already.

  // Return path if it was altered by any module OR it already includes base
  if ( ($path != $old_path) || (strpos($path, '/') === 0) ) {
    return $path;
  }

Attached is the modified core path. It is trivial - only changing the IF line above (and updating the comment).

Sorry, wrong patch. This one should do it...

I have not had chance to review your new patches.

So far I just upgraded to the latest dev version of this module with my auto-build script and it is working for my use cases so far.

What do you want me to actually review? I will surely take a deeper look if my current setup breaks :)

OK, with pathologic module being forcefully upgraded to 3.x, I had to reapply/retest all patches.

Pathologic module patch had to be rewritten since that module's functions changed. I attached the rewritten and streamlined patch. It might use further work, as I only implemented a bare minimum regex that might not cover all use cases. Reporting the patch to pathologic issue queue.

Basic CDN patches from the module distribution applied just fine and everything seems to be working as far as I tested.

One minor annoyance is the CDN module's stats page shows original URLs, not rewritten / post-processed one. I have not tackled that code but suggest this as enhancement.

OK, a little addition here. Looks like there is need to update your CDN patch for drupal core (attached).

It has only one minor difference from your original one. In file.inc, if the path starts with leading '/' this means the base path was already prepended. If this fix is not present many modules that call url function repeatedly will append double '//' in front of the paths. Here is the revised snippet:

  // Return path if it was altered by any module OR it already includes base
  if ( ($path != $old_path) || (strpos($path, '/') === 0) ) {
    return $path;
  }

This fixed numerous URL-generation bugs I had in several sites.