mpratt / Embera

A Oembed consumer library, that gives you information about urls. It helps you replace urls to youtube or vimeo for example, with their html embed code. It has advanced features like offline support, responsive embeds and caching support.
MIT License
335 stars 59 forks source link

Update HttpRequest.php #11

Closed bruno-schmidt closed 10 years ago

bruno-schmidt commented 10 years ago

the $options variable was not being updated after checking safe_mode and open_basedir.

mpratt commented 10 years ago

Hi Annieyo!

Thanks for your pull request, I really appreciate it.

You indeed have found a bug! In fact, you made me realize that I made a mistake on this line too:

        $options = $this->config['curl'] + array(
            CURLOPT_USERAGENT => $this->userAgent,
            CURLOPT_ENCODING => '',
        ) + $params;

The order is messed up! What I really wanted was to have some default values as a base, then use $this->config['curl'] to override or add more values into curl and then use the $params array to override/add custom values. It could be useful in case a service needs something extra/weird.

So the order should be:

        $options = $params + $this->config['curl'] + array(
            CURLOPT_USERAGENT => $this->userAgent,
            CURLOPT_ENCODING => '',
        );

Anyways!

How do you feel about using the original class and just changing the whole section with:

        // CURLOPT_FOLLOWLOCATION doesnt play well with open_basedir/safe_mode
        if (ini_get('safe_mode') || ini_get('open_basedir')) {
            $options[CURLOPT_FOLLOWLOCATION] = false;
            $options[CURLOPT_TIMEOUT] = 15;
            $this->config['force_redirects'] = true;
        }

And why not take it a step further and replace the line 117 with

    return $this->curl($url, $options);

Let me know what you think!

I believe this way is a little more straightforward. If you agree (and have time), can you send another pull request with the above changes so I can merge them and tag another release?

Otherwise, perhaps my Idea is not that great so please let me know!

If I dont hear from you in a couple of days, then I will write the changes myself and give you credit on the commit log :(

Thank you!

bruno-schmidt commented 10 years ago

Hello!

It's nicer this way! I will send another pull request, if I forget something, please feel free to edit anything you need!

I'm new to git, so tell me if I sent it wrong haha

Thank you!