Absolute urls inserted with CDN module enabled [#2001284]

With insert_absolute_paths variable set to FALSE, absolute urls will still be inserted when the CDN module is enabled. See #1863530: Don't generate CDN URLs when using the Insert module by blacklisting file/ajax/*. I've created a patch that strips off the domain if the original uri includes a local stream wrapper. I *think* it's safe, but I'd like some feedback.

Comment	File	Size	Author
	insert-cdn.patch	852 bytes	ttkaminski

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

quicksketch

he/him

CreditAttribution: quicksketch commented 27 May 2013 at 07:56

This patch definitely does not feel very safe. It seems very strange that you would be saying, "if this is a local file, but the URL is remote, then remove the domain". Of course the situation that the file would be local and the URL be remote is strange in itself, but I suppose that's the way CDN module operates. I'm not very comfortable with this patch on the whole. Even with additional documentation around these calls, it's introducing some pretty messy situational code that seems like it could have unexpected results.

Comment #2

ttkaminski CreditAttribution: ttkaminski commented 27 May 2013 at 14:01

I agree that there may be some fringe cases where this patch may cause problems, but I can't think of any specific examples ATM. The drupal function file_create_url does not accept any argument for requesting a local (relative url). I think this would be the best approach: $url = file_create_url($uri, array("absolute" => FALSE));, but this would involve first patching the core.

In the meantime, I've been using the above patch in production for a few days without any noticeable problems. I wouldn't feel bad if it wasn't merged in, but it's definitely useful for people who need it.