New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined function http_response_code() #2

Closed
pongo opened this Issue Oct 8, 2014 · 16 comments

Comments

Projects
None yet
3 participants
@pongo

pongo commented Oct 8, 2014

http_response_code() function was introduced in PHP 5.4. For lower version of PHP you can include this function http://php.net/manual/en/function.http-response-code.php#107261

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Oct 8, 2014

Owner

Unfortunately, that solution is a little broken. It doesn't detect, for example, opencart doing this:

header('HTTP/1.1 404 Not Found');

It will just return "200".

Nonetheless, I'll see if I can come up with a better solution. Probably parsing headers_list().

Owner

budgetneon commented Oct 8, 2014

Unfortunately, that solution is a little broken. It doesn't detect, for example, opencart doing this:

header('HTTP/1.1 404 Not Found');

It will just return "200".

Nonetheless, I'll see if I can come up with a better solution. Probably parsing headers_list().

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Oct 8, 2014

Owner

Sadly, there's not a good solution for this.

We use http_response_code() to ensure that we only cache responses with a status of "200 OK". This keeps us from, for example, caching 5XX server errors (that might be temporary), 404 Not Found, etc.

PHP5.3 does not allow any real way to get the http status of the current request. For example, the status line, even though it's a header, is specifically hidden in the headers_list() call.

I tried to get the information I needed from opencart. That initially seemed promising, as we already have the opencart response object in the code. Sadly, while the response object does have access to the data I need, it's marked as a private attribute.

All that said, I did just push a change so that it will run on PHP5.3. What it does now is only leverage http_response_code() if it's available. That means, though, if you run it on PHP5.3, it will cache 404 errors, 5XX errors, etc. If http_response_code() is available, it will only cache responses that are "200 OK".

I do not recommend running this under PHP5.3. See the "Requirements" section of readme.md for details.

Owner

budgetneon commented Oct 8, 2014

Sadly, there's not a good solution for this.

We use http_response_code() to ensure that we only cache responses with a status of "200 OK". This keeps us from, for example, caching 5XX server errors (that might be temporary), 404 Not Found, etc.

PHP5.3 does not allow any real way to get the http status of the current request. For example, the status line, even though it's a header, is specifically hidden in the headers_list() call.

I tried to get the information I needed from opencart. That initially seemed promising, as we already have the opencart response object in the code. Sadly, while the response object does have access to the data I need, it's marked as a private attribute.

All that said, I did just push a change so that it will run on PHP5.3. What it does now is only leverage http_response_code() if it's available. That means, though, if you run it on PHP5.3, it will cache 404 errors, 5XX errors, etc. If http_response_code() is available, it will only cache responses that are "200 OK".

I do not recommend running this under PHP5.3. See the "Requirements" section of readme.md for details.

@budgetneon budgetneon closed this Oct 8, 2014

@pongo

This comment has been minimized.

Show comment
Hide comment
@pongo

pongo Oct 8, 2014

Why we can't open system/library/response.php and make $headers array public?

pongo commented Oct 8, 2014

Why we can't open system/library/response.php and make $headers array public?

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Oct 8, 2014

Owner

Most of the major hosting companies have switched to PHP5.4, as have the major linux distributions, including Debian, which is usually slow to adopt new versions.

Also see this: http://forum.opencart.com/viewtopic.php?f=24&t=130933

I don't want to have to add a dependency on vqmod, or more changes to core opencart files. I'd like to keep this page cache relatively simple.

That said, yes, it would be fairly easy for anyone to make that change to response.php and then a few small changes in this code.

Owner

budgetneon commented Oct 8, 2014

Most of the major hosting companies have switched to PHP5.4, as have the major linux distributions, including Debian, which is usually slow to adopt new versions.

Also see this: http://forum.opencart.com/viewtopic.php?f=24&t=130933

I don't want to have to add a dependency on vqmod, or more changes to core opencart files. I'd like to keep this page cache relatively simple.

That said, yes, it would be fairly easy for anyone to make that change to response.php and then a few small changes in this code.

@pongo

This comment has been minimized.

Show comment
Hide comment
@pongo

pongo Oct 8, 2014

My hosting have only 5.3. I change response.php, what i need change in pagecache.php?

pongo commented Oct 8, 2014

My hosting have only 5.3. I change response.php, what i need change in pagecache.php?

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Oct 8, 2014

Owner

I'll see if I have time to make a one-off pagecache.php for you, but I can't do it today. Maybe tomorrow or friday. Thanks..

Owner

budgetneon commented Oct 8, 2014

I'll see if I have time to make a one-off pagecache.php for you, but I can't do it today. Maybe tomorrow or friday. Thanks..

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Oct 9, 2014

Owner

Okay...

  1. You can put the headers attribute in response.php back to private.

  2. Add the following function into response.php:

public function http_response_code() {
    foreach ($this->headers as $header) {
        if (preg_match("#^HTTP/\S+\s+(\d\d\d)#i",$header,$matches)) {
            return $matches[1];
        }
    }
    return 200;
}
  1. Within pagecache.php, change this:
    if (function_exists('http_response_code')) {
        if (http_response_code() != 200) {
            return false;
        }
    }

To this:

   if (method_exists($response,'http_response_code')) {
        if ($response->http_response_code() != 200) {
            return false;
        }
    }
Owner

budgetneon commented Oct 9, 2014

Okay...

  1. You can put the headers attribute in response.php back to private.

  2. Add the following function into response.php:

public function http_response_code() {
    foreach ($this->headers as $header) {
        if (preg_match("#^HTTP/\S+\s+(\d\d\d)#i",$header,$matches)) {
            return $matches[1];
        }
    }
    return 200;
}
  1. Within pagecache.php, change this:
    if (function_exists('http_response_code')) {
        if (http_response_code() != 200) {
            return false;
        }
    }

To this:

   if (method_exists($response,'http_response_code')) {
        if ($response->http_response_code() != 200) {
            return false;
        }
    }
@pongo

This comment has been minimized.

Show comment
Hide comment
@pongo

pongo Oct 9, 2014

Thank you very much! All works.

pongo commented Oct 9, 2014

Thank you very much! All works.

@budgetneon budgetneon self-assigned this Oct 12, 2014

@adrianmihalko

This comment has been minimized.

Show comment
Hide comment
@adrianmihalko

adrianmihalko Dec 15, 2014

In the response.php the code goes into the "class Response { }" ?

adrianmihalko commented Dec 15, 2014

In the response.php the code goes into the "class Response { }" ?

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Dec 16, 2014

Owner

Yes, it's a new method within the class.

You could, for example, put it in just after:

    public function addHeader($header) {
        $this->headers[] = $header;
    }

That said, I would highly recommend just upgrading to PHP5.4.

Php 5.3 is officially end of life, and no longer supported.

http://php.net/eol.php

Owner

budgetneon commented Dec 16, 2014

Yes, it's a new method within the class.

You could, for example, put it in just after:

    public function addHeader($header) {
        $this->headers[] = $header;
    }

That said, I would highly recommend just upgrading to PHP5.4.

Php 5.3 is officially end of life, and no longer supported.

http://php.net/eol.php

@adrianmihalko

This comment has been minimized.

Show comment
Hide comment
@adrianmihalko

adrianmihalko Dec 16, 2014

Yes I want to upgrade, but my hosting company want's to upgrade the whole OS on my VPS. The OC is fully PHP 5.4 compatible?

I would be very grateful if you could confirm my manual pagecache installation in index.php:

http://pastebin.ca/2887517

adrianmihalko commented Dec 16, 2014

Yes I want to upgrade, but my hosting company want's to upgrade the whole OS on my VPS. The OC is fully PHP 5.4 compatible?

I would be very grateful if you could confirm my manual pagecache installation in index.php:

http://pastebin.ca/2887517

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Dec 16, 2014

Owner

In your pastebin, there's an issue. It should look like this at the bottom:

// Output
$response->output();   

if ($pagecache->OkToCache()) {                                  //PAGECACHE
    $pagecache->CachePage($response);                           //PAGECACHE
}                                                               //PAGECACHE

You have the order reversed.

Owner

budgetneon commented Dec 16, 2014

In your pastebin, there's an issue. It should look like this at the bottom:

// Output
$response->output();   

if ($pagecache->OkToCache()) {                                  //PAGECACHE
    $pagecache->CachePage($response);                           //PAGECACHE
}                                                               //PAGECACHE

You have the order reversed.

@adrianmihalko

This comment has been minimized.

Show comment
Hide comment
@adrianmihalko

adrianmihalko Dec 16, 2014

Thank you, I fixed that.

My final pagecache.php:

     if (method_exists($response,'http_response_code')) {
        if ($response->http_response_code() != 200) {
            if (function_exists('php_sapi_name')) {
                // litespeed is broken, http_response_code()
                // returns '' when it should return 200
                if (php_sapi_name() == 'litespeed') {
                    if (http_response_code() != '') {
                        return false;
                    }
                }
            } else { 
                return false;
            }     
        }
    }

adrianmihalko commented Dec 16, 2014

Thank you, I fixed that.

My final pagecache.php:

     if (method_exists($response,'http_response_code')) {
        if ($response->http_response_code() != 200) {
            if (function_exists('php_sapi_name')) {
                // litespeed is broken, http_response_code()
                // returns '' when it should return 200
                if (php_sapi_name() == 'litespeed') {
                    if (http_response_code() != '') {
                        return false;
                    }
                }
            } else { 
                return false;
            }     
        }
    }
@adrianmihalko

This comment has been minimized.

Show comment
Hide comment
@adrianmihalko

adrianmihalko Dec 18, 2014

Hmmm, I just tested today, but 404 pages are still cached.

adrianmihalko commented Dec 18, 2014

Hmmm, I just tested today, but 404 pages are still cached.

@budgetneon

This comment has been minimized.

Show comment
Hide comment
@budgetneon

budgetneon Dec 18, 2014

Owner

Looks like there's some missing logic, yes.

Change your snipped above to:

 if (method_exists($response,'http_response_code')) {
    if ($response->http_response_code() != 200) {
        if (function_exists('php_sapi_name')) {
            // litespeed is broken, http_response_code()
            // returns '' when it should return 200
            if (php_sapi_name() == 'litespeed') {
                if (http_response_code() != '') {
                    return false;
                }
            } else {
                return false;
            }
        } else { 
            return false;
        }     
    }
}

And, also, clear the cache. It will serve the cached file if it's sitting there and not expired.

Owner

budgetneon commented Dec 18, 2014

Looks like there's some missing logic, yes.

Change your snipped above to:

 if (method_exists($response,'http_response_code')) {
    if ($response->http_response_code() != 200) {
        if (function_exists('php_sapi_name')) {
            // litespeed is broken, http_response_code()
            // returns '' when it should return 200
            if (php_sapi_name() == 'litespeed') {
                if (http_response_code() != '') {
                    return false;
                }
            } else {
                return false;
            }
        } else { 
            return false;
        }     
    }
}

And, also, clear the cache. It will serve the cached file if it's sitting there and not expired.

@adrianmihalko

This comment has been minimized.

Show comment
Hide comment
@adrianmihalko

adrianmihalko Dec 18, 2014

Superb, it is working now. 👍

adrianmihalko commented Dec 18, 2014

Superb, it is working now. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment