onPHP / onphp-framework

onPHP is the mature GPL'ed multi-purpose object-oriented PHP framework.
85 stars 52 forks source link

Отправка файлов через Curl #226

Open DeryabinSergey opened 9 years ago

DeryabinSergey commented 9 years ago

У нас сейчас в CurlHttpClient при отправке файлов массивчик с файликами присоединяется к POST и при этом проверяется что бы в остальных значениях не было в начале собаки.

Что имеем сейчас, появился параметр CURLOPT_SAFE_UPLOAD:

TRUE to disable support for the @ prefix for uploading files in CURLOPT_POSTFIELDS, which means that values starting with @ can be safely passed as fields. CURLFile may be used for uploads instead. Added in PHP 5.5.0 with FALSE as the default value. PHP 5.6.0 changes the default value to TRUE.

И про CURLOPT_POSTFIELDS:

As of PHP 5.5.0, the @ prefix is deprecated and files can be sent using CURLFile. The @ prefix can be disabled for safe passing of values beginning with @ by setting the CURLOPT_SAFE_UPLOAD option to TRUE.

Смотреть здесь: http://ru.php.net/manual/en/function.curl-setopt.php (именно на английском! в русккой версии документации не упоминается)

Сам же http://ru.php.net/manual/en/class.curlfile.php идет только с версии 5.5.0 Хотя есть еще такие кто на 5.2.хх сидят - сам вот только переползаю, очень трудоемкий процесс получается )))

Какое можно элегантное решение придумать?

AlexeyDsov commented 9 years ago

Думал я в master вносил это исправление давным давно, видимо хотел сделать, забыл, а запомнил что проблема решена. Вот так выглядит решение в ветке masterNs

DeryabinSergey commented 9 years ago

Нормальный вариант в принципе. Без разбивки по версиям более элегантное вряд ли что нить придумается. Тогда уж если версии проверять, можно и в makeHandle сделать проверку и для версии от 5.5 добавить CURLOPT_SAFE_UPLOAD и не вызывать findAtParamInPost

DeryabinSergey commented 9 years ago

И еще небольшое замечание по отправке файлов - если отправлять файлы через CURLFile при отсутствующем файле бросается уже не WrongArgumentException а NetworkException и текст ошибки там другой. Соответственно и тест CurlHttpClientTest::testSendingNotExistsFile() ломается. И в проектах у кого-то может поломаться при использовании

DeryabinSergey commented 9 years ago

И еще что-то не то с Curl у меня, у вас тесты нормально отрабатывают? У меня не проходит тест CurlHttpClientTest с отправкой запроса в теле.

            $body = file_get_contents($this->getFile1Path());
            $request3 = $this->spawnRequest(HttpMethod::get())->
                setBody($body);
            $this->assertEquals(
                $this->generateString(array(), array(), array(), $body),
                $client->getResponse($request3)->getBody()
            );

У меня почему-то $body еще отправляется в POST ассоциативным массивом с ключом из $body и пустым значением. Вот простой примерчик, что получается, если даже без onPHP делать:

$ch = curl_init();
$data = file_get_contents('/var/www/onPHP.master/test/main/data/directory/contents');
curl_setopt($ch, CURLOPT_URL, 'http://localhost/curlTest.php');
curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
echo curl_exec($ch);

На выходе:

array(0) {
}
array(1) {
  ["--
Truly_thours,
Queen_of_Britan
"]=>
  string(0) ""
}
array(0) {
}
string(33) "--
Truly thours,
Queen of Britan
"
AlexeyDsov commented 9 years ago

Создал #227 - Pull Request с исправлениями. На 5.4 правда проверить сейчас не могу, т.к. нет под рукой, но, вроде бы должно все работать без изменений.

@DeryabinSergey Про $body дублирующемся в POST - подозреваю что есть какая-то настройка в php на этот случай. У меня такой кейс не воспроизводится. Кажется, когда-то давно даже с этим сталкивался, но пока не вспомнил что это.

DeryabinSergey commented 9 years ago

@AlexeyDsov а ты на какой версии пыха сидишь?

AlexeyDsov commented 9 years ago

5.5, все еще грешу что дело в настройке php. Или, кстати, веб сервера.

DeryabinSergey commented 9 years ago

У меня на nginx крутится все. Перекопал уже ВСЕ параметры в php.ini - ничего подходящего не нашел

AlexeyDsov commented 9 years ago

Посмотрел настройки php.ini и тоже не нашел. Не знаю почему оно так сейчас, но помню такой случай. Впринципе то сильно важно что оно немного странно работает? Что бы убедиться что дело именно в curl, а не в CurlHttpClient'е то можно постучаться telnet'ом на него:

telnet 127.0.0.1 80
POST / HTTP/1.1
Host: curl.test
Content-Length: 20

12345678901234567890

Не забыть после последнего символа (0) нажать ввод. И адрес хоста и ip/port для telnet нужно ,конечно же, соотвествующие подставить. Скрипт тестовый должен поймать только боди (3-ий элемент массива). У меня так:

Array
(
    [0] => Array
        (
        )

    [1] => Array
        (
        )

    [2] => Array
        (
        )

    [3] => 12345678901234567890
)

Соотвественно в вашем случае должно еще и в пером оказаться 12345678901234567890 => "". В случае если так и есть - значит настройки веб сервера и php. Если только в body - тогда надо будет посмотреть что такого может делать CurlHttpClient.

DeryabinSergey commented 9 years ago

В принципе все логично отрабатывает:

telnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
POST /curlTest.php HTTP/1.1    
Host: 127.0.0.1
Content-Length: 20

12345678901234567890
HTTP/1.1 200 OK
Server: nginx/1.4.6 (Ubuntu)
Date: Tue, 18 Nov 2014 09:11:53 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive

49
array(0) {
}
array(0) {
}
array(0) {
}
string(20) "12345678901234567890"

Но если я добавлю в заголовок application/x-www-form-urlencoded, который по идее и добавляется в тесте при пост запросе: http://php.net/manual/ru/function.curl-setopt.php#refsect1-function.curl-setopt-notes там же мы по сути как раз строку и передаем, а не массив. То у меня так и получается:

telnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
POST /curlTest.php HTTP/1.1
Host: 127.0.0.1
Content-Length: 20
Content-type: application/x-www-form-urlencoded

12345678901234567890
HTTP/1.1 200 OK
Server: nginx/1.4.6 (Ubuntu)
Date: Tue, 18 Nov 2014 09:14:05 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive

75
array(0) {
}
array(1) {
  ["12345678901234567890"]=>
  string(0) ""
}
array(0) {
}
string(20) "12345678901234567890"
AlexeyDsov commented 9 years ago

Забавно, но у меня такой вариант все так же только в body ловит не смотря на добавленны заголовок Content-type: application/x-www-form-urlencoded

Но вот если само тело заменить на: 12345=789&09876=4321, то POST уже становится не пустым

DeryabinSergey commented 9 years ago

Точно так же у меня на фронте, где PHP 5.2.X А на локалке на 5.3 и 5.6 как описываю.

telnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
POST /curlTest.php HTTP/1.1
Host: 127.0.0.1
Content-Length: 19
Content-type: application/json

{"test-data":"arr"}
HTTP/1.1 200 OK
Server: nginx/1.4.6 (Ubuntu)
Date: Tue, 18 Nov 2014 09:34:08 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive

48
array(0) {
}
array(0) {
}
array(0) {
}
string(19) "{"test-data":"arr"}"
telnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
POST /curlTest.php HTTP/1.1
Host: 127.0.0.1
Content-type: text/xml
Content-Length: 55

<?xml version="1.0"?><greeting>Hello, world!</greeting>
HTTP/1.1 200 OK
Server: nginx/1.4.6 (Ubuntu)
Date: Tue, 18 Nov 2014 09:38:35 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive

6c
array(0) {
}
array(0) {
}
array(0) {
}
string(55) "<?xml version="1.0"?><greeting>Hello, world!</greeting>"

Мне кажется у меня более логичное поведение ) Но что этот один тест не отрабатывает меня в принципе особо пока не напрягает

DeryabinSergey commented 9 years ago

И вот еще что, это только для POST, при скажем PUT запросе такого тоже нет

telnet 127.0.0.1 80
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
PUT /curlTest.php HTTP/1.1
Host: 127.0.0.1
Content-Length: 20
Content-type: application/x-www-form-urlencoded

12345678901234567890
HTTP/1.1 200 OK
Server: nginx/1.4.6 (Ubuntu)
Date: Tue, 18 Nov 2014 09:42:19 GMT
Content-Type: text/html; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive

49
array(0) {
}
array(0) {
}
array(0) {
}
string(20) "12345678901234567890"
AlexeyDsov commented 9 years ago

В случае PUT у меня скрипт не ловит в POST ничего даже если передать body aaaa=bbbb&dddd=bbbbb

DeryabinSergey commented 9 years ago

А еще такой момент, у нас 9 DAO воркеров, в тесте у меня вываливает 8 ошибок:

There were 8 errors:

1) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

2) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

3) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

4) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

5) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

6) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

7) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

8) AutoloaderClassPathCacheTest::testWithBaseException
BaseException: Constant DEBUG already defined

/var/www/onPHP.master/global.inc.php.tpl:16
/var/www/onPHP.master/test/main/Autoloader/AutoloaderClassPathCacheTest.class.php:51

Они не перехватываются что-ли или почему так?

AlexeyDsov commented 9 years ago

Ох :). См. #221, 0e4d455

DeryabinSergey commented 9 years ago

Спасибо. Мержите уже скорее )))

И еще хотелось бы к Curl вернуться. CurlHttpClient::makeHandle по сути получается работает только с GET и POST запросами. Если делать PUT DELETE INSERT и прочее - ничего не получится же. Т. к. там сейчас

                default:
                    $options[CURLOPT_CUSTOMREQUEST] = $request->getMethod()->getName();
                    break;

Не уверен, насчет queryString но body нужен как минимум

                default:
                    $options[CURLOPT_CUSTOMREQUEST] = $request->getMethod()->getName();
                    if ($request->hasBody()) {
                                                $options[CURLOPT_POSTFIELDS] = $request->getBody();
                    }
                    break;
AlexeyDsov commented 9 years ago

Согласен. Так было и раньше. Я ими не пользовался никогда толком и когда правил CurlHttpClient то, вроде бы не трогал. Для меня они все так же экзотика :) Если есть видение как оно должно быть - можно поправить. Но у меня виденья нету. Имеющиеся вопросы к другим методам:

Эксперимент с telnet'ом описанный выше показал что мой локальный вебсервер с какими-то более менее дефолтными настройками при PUT body на POST параметры не бьёт. Но должен ли считаться с этим ограничением CurlHttpClient? Я не уверен, т.к., думаю, вебсервер можно настроить и что б бил на параметры body любого типа запроса.

DeryabinSergey commented 9 years ago

У меня видения как такового тоже нет, но беглый просмотр API Google (календарь первым попался) https://developers.google.com/google-apps/calendar/v3/reference/acl/update показывает что GET параметры в URL нужны, там просто URL формируется уже в зависимости от параметров, а так в принципе те же параметры могут просто явно передаваться.

POST параметров быть не должно и не может, т. к. смысла в них нет, они все равно не распарсятся в массив _POST на принимающей стороне, а должно быть либо BODY либо ничего. Т.к. таких запросах гоняют или XML или JSON, как правило.

Готов постоять послушать другие мнения, или же подождать пока кому нить не понадобится эта реализация

AlexeyDsov commented 9 years ago

Исходя из того что я выше написал и @DeryabinSergey, я б предложил бы что стоит сделать поведение всех не GET методов таким же как поведение при POST. А ставить body или post параметры оставить на усмотрение того кто будет использовать.

DeryabinSergey commented 4 years ago

@DeryabinSergey Про $body дублирующемся в POST - подозреваю что есть какая-то настройка в php на этот случай. У меня такой кейс не воспроизводится. Кажется, когда-то давно даже с этим сталкивался, но пока не вспомнил что это.

@AlexeyDsov прошло 6 лет ) Добавил проверку тревисом и такая же ерунда - BODY подставляется в POST

DeryabinSergey commented 4 years ago

Такой уже наверное больше философский вопрос, а надо ли поддерживать такие обертки как CurlHttpClient когда есть https://github.com/guzzle/guzzle с гораздо большим функционалом, асинхронными запросами и прочими плюшками