Hello,

I recently traced an issue on the amazons3_cors to a conflict between Libraries and the version of the AWS SDK that is supported by the most commonly used AWS modules for Drupal (here: https://drupal.org/node/1968798#comment-7516515).

In short, the old AWS SDK unsets the $path variable in the global scope, which libraries_load_files() uses to build the path to the require_once on line 707.

This patch fixes that problem for that particular library by changing the name of the $path variable to $library_path.

Ideally we could all just use higher-quality libraries. I'm definitely not sending a patch which caches all defined variables before the require_once and restores them after, because that would be very silly. This is only slightly silly.

CommentFileSizeAuthor
#4 2015731-4-tests-only.patch2.84 KBtstoeckler
#3 2015731-3.patch3.17 KBtstoeckler
#3 2015731-1-3-interdiff.txt1.96 KBtstoeckler
#1 2015731-1-require_once-protection.patch1.46 KBtstoeckler
fixes-amazons3_cors_libraries_info_alter-bug-1968798-7516515.patch1.4 KBCaptainHook-1
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Title: amazons3_cors/awssdk support issue » Add protection for library files overriding local variables in libraries_load_files()
Issue tags: +Needs tests
FileSize
2015731-1-require_once-protection.patch1.46 KB

I just stumbled over this issue again. While there's nothing inherently problematic with your patch I wonder if we can find a more generic / safe solution. I agree, however, that caching variables is not a viable solution.

Can you try the attached patch? Unless I'm mistaken it solves the issue in a minimally hackish way. Thanks.

I guess we should also provide tests for this.

amarnus’s picture

amarnus CreditAttribution: amarnus commented

I like #1. Was exactly what I thought of when I hit the same issue with amazons3_cors.

Actually, for PHP 5.3+, we can just do:

call_user_func(function() use ($filename) { require_once($filename); });

For all versions, the wrapper function approach is safest, of course.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
FileSize
2015731-1-3-interdiff.txt1.96 KB
2015731-3.patch3.17 KB

Let's see.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
FileSize
2015731-4-tests-only.patch2.84 KB

A tests-only patch can't hurt, maybe.

Status: Needs review » Needs work

The last submitted patch, 2015731-4-tests-only.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Fixed

Committed this. Thanks!

7.x-2.x: http://drupalcode.org/project/libraries.git/commit/1e03f472a3600a6529a7e...
7.x-3.x: http://drupalcode.org/project/libraries.git/commit/5a8f12ee89166b0256db4...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Issue summary: View changes

Fixed "on on" to "on"

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.