Izumi-kun / LTI-Tool-Provider-Library-PHP

LTI Tool Provider library for PHP
Apache License 2.0
21 stars 6 forks source link

StreamClient does not populate $message reponse properties #9

Closed jozefbriss closed 6 years ago

jozefbriss commented 6 years ago

https://github.com/Izumi-kun/LTI-Tool-Provider-Library-PHP/blob/aa68a432aa733dc73db4a8c5388482126f4fa2a1/src/Http/StreamClient.php#L12

public function send(HTTPMessage $message)
{
    $opts = [
        'method' => $message->method,
        'content' => $message->request,
        'ignore_errors' => true
    ];
    if (!empty($message->requestHeaders)) {
        $opts['header'] = $message->requestHeaders;
    }
    try {
        $ctx = stream_context_create(['http' => $opts]);
        $fp = @fopen($message->url, 'rb', false, $ctx);
        if ($fp) {
            $resp = @stream_get_contents($fp);
            $ok = $resp !== false;
            if ($ok) {
                $message->response = $resp;
                if (isset($http_response_header[0])) {
                    $message->responseHeaders = $http_response_header;
                    if( preg_match( "/HTTP\/\d.\d\s+(\d+)/", $message->responseHeaders[0], $out )) {
                        $message->status = $out[1];
                    }
                    $ok = $message->status < 400;
                    if (!$ok) {
                        $message->error = $message->responseHeaders[0];
                    }
                }
                return $ok;
            }
        }
    } catch (\Exception $e){
        $message->error = $e->getMessage();
        return false;
    }
    $message->error = error_get_last()["message"];
    return false;
}

My default PHP HTTP wrapper requires "Accept" header e.g. "Accept: /" otherwise it will return "HTTP/1.1 406 Not Acceptable" so maybe part of client functionality should be that at adding "Accept: /" if no accept header exists.

jozefbriss commented 6 years ago

There are missing star characters in Accept header examples in the previous comment. Maybe due to this editor.

jozefbriss commented 6 years ago

Curl adds Accept header if missing. Curl $message->requestHeaders and $message->responseHeaders are string and it is array for StreamClient

jozefbriss commented 6 years ago

Here is version better compatible with what curl does:

public function send(HTTPMessage $message)
{
    if (empty($message->requestHeaders)) {
        $message->requestHeaders = ["Accept: */*"];
    } elseif (count(preg_grep("/^Accept:/", $message->requestHeaders)) == 0) {
        $message->requestHeaders[] = "Accept: */*";
    }
    $opts = [
        'method' => $message->method,
        'content' => $message->request,
        'header' => $message->requestHeaders,
        'ignore_errors' => true
    ];

    $message->requestHeaders = implode("\n", $message->requestHeaders);
    try {
        $ctx = stream_context_create(['http' => $opts]);
        $fp = @fopen($message->url, 'rb', false, $ctx);
        if ($fp) {
            $resp = @stream_get_contents($fp);
            $ok = $resp !== false;
            if ($ok) {
                $message->response = $resp;
                if (isset($http_response_header[0])) {
                    $message->responseHeaders = implode("\n", $http_response_header);
                    if( preg_match( "/HTTP\/\d.\d\s+(\d+)/", $http_response_header[0], $out )) {
                        $message->status = $out[1];
                    }
                    $ok = $message->status < 400;
                    if (!$ok) {
                        $message->error = $http_response_header[0];
                    }
                }
                return $ok;
            }
        }
    } catch (\Exception $e){
        $message->error = $e->getMessage();
        return false;
    }
    $message->error = error_get_last()["message"];
    return false;
}