[PHP-INTERNALS] ext/curl safe_upload is changed

[PHP-INTERNALS] ext/curl safe_upload is changed

	Laruence
	May 9, 2014 at 5:48 am

Hey Stas:

      I noticed that this change : https://github.com/php/php-src/commit/24447dca

      it has broken some tests in ext/curl... did you run make test before?

      Anyway, the current behavior is silent return if the codes uses
old upload style("@filename");

      I think it's better to throw a warning or something error about that....

      thoughts?

thanks

--
Laruence Xinchen Hui
http://www.laruence.com/

Search Discussions

3 responses
Oldest
Nested

Stas Malyshev

at May 9, 2014 at 6:52 am

⇧

Hi!

I noticed that this change : https://github.com/php/php-src/commit/24447dca

it has broken some tests in ext/curl... did you run make test before?
Which tests? AFAIK the tests on Travis include curl, and they are green.
Anyway, the current behavior is silent return if the codes uses
old upload style("@filename");

What you mean by silent return? If unsafe upload is not turned on,
@filename is just a variable like any other.

I think it's better to throw a warning or something error about that....

I don't think so. "@filename" is a string like any others, there's no
reason to throw warnings on legitimate upload values.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

reply | permalink

Laruence

at May 9, 2014 at 6:54 am

⇧

On Fri, May 9, 2014 at 2:52 PM, Stas Malyshev wrote:
Hi!

I noticed that this change : https://github.com/php/php-src/commit/24447dca

it has broken some tests in ext/curl... did you run make test before?

Which tests? AFAIK the tests on Travis include curl, and they are green.

$ cat ext/curl/tests/bug27023.diff
001+ string(0) ""
002+ string(0) ""
003+ string(0) ""
004+ string(0) ""
005+ string(0) ""
001- string(%d) "curl_testdata1.txt|application/octet-stream"
002- string(%d) "curl_testdata1.txt|text/plain"
003- string(%d) "foo.txt|application/octet-stream"
004- string(%d) "foo.txt|text/plain"
005- string(%d) "foo.txt|text/plain"

thanks

Anyway, the current behavior is silent return if the codes uses
old upload style("@filename");

What you mean by silent return? If unsafe upload is not turned on,
@filename is just a variable like any other.

I think it's better to throw a warning or something error about that....

I don't think so. "@filename" is a string like any others, there's no
reason to throw warnings on legitimate upload values.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

--
Laruence Xinchen Hui
http://www.laruence.com/

reply | permalink

Stas Malyshev

at May 9, 2014 at 7:06 am

⇧

Hi!

Which tests? AFAIK the tests on Travis include curl, and they are green.

$ cat ext/curl/tests/bug27023.diff

Ahh, this test requires special configuration. It'd be better to rewrite
it to supply its own server and not to have to set up special server for
it. But it needs either:
curl_setopt($ch, CURLOPT_SAFE_UPLOAD, 0);
or, even better, to be rewritten with new safe API in mind. Preferably
the latter. I'll look into it on the weekend if nobody beats me to it.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

reply | permalink

Related Discussions

Discussion Navigation

view	thread \| post

Discussion Overview

group	php-internals @
categories	php
posted	May 9, '14 at 5:48a
active	May 9, '14 at 7:06a
posts	4
users	2
website	php.net

responses	expanded	Hotkey: s
font	variable	Hotkey: f
user style	avatars	Hotkey: a

[PHP-INTERNALS] ext/curl safe_upload is changed

Search Discussions

Related Discussions

2 users in discussion