Skip to content
Browse files

- Implemented FR #50692, not uploaded files don't count towards

  max_file_uploads limit.
- As a side improvement, temporary files are not opened for
  empty uploads and, in debug mode, 0-length uploads.
  • Loading branch information...
1 parent 090a9b3 commit 817c28c867f6bc0b490857013d0b4fa71a467082 @cataphract cataphract committed Oct 4, 2010
View
3 NEWS
@@ -12,6 +12,9 @@
- Implemented symbolic links support for open_basedir checks. (Pierre)
- Implemented FR #51804, SplFileInfo::getLinkTarget on Windows. (Pierre)
+- Implemented FR #50692, not uploaded files don't count towards
+ max_file_uploads limit. As a side improvement, temporary files are not opened
+ for empty uploads and, in debug mode, 0-length uploads. (Gustavo)
- Fixed possible flaw in open_basedir (CVE-2010-3436). (Pierre)
- Fixed MOPS-2010-24, fix string validation. (CVE-2010-2950). (Pierre)
View
45 main/rfc1867.c
@@ -1011,16 +1011,8 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
}
total_bytes = cancel_upload = 0;
-
- if (!skip_upload) {
- /* Handle file */
- fd = php_open_temporary_fd_ex(PG(upload_tmp_dir), "php", &temp_filename, 1 TSRMLS_CC);
- upload_cnt--;
- if (fd == -1) {
- sapi_module.sapi_error(E_WARNING, "File upload error - unable to create a temporary file");
- cancel_upload = UPLOAD_ERROR_E;
- }
- }
+ temp_filename = NULL;
+ fd = -1;
if (!skip_upload && php_rfc1867_callback != NULL) {
multipart_event_file_start event_file_start;
@@ -1029,13 +1021,6 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
event_file_start.name = param;
event_file_start.filename = &filename;
if (php_rfc1867_callback(MULTIPART_EVENT_FILE_START, &event_file_start, &event_extra_data TSRMLS_CC) == FAILURE) {
- if (temp_filename) {
- if (cancel_upload != UPLOAD_ERROR_E) { /* file creation failed */
- close(fd);
- unlink(temp_filename);
- }
- efree(temp_filename);
- }
temp_filename = "";
efree(param);
efree(filename);
@@ -1058,7 +1043,26 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
offset = 0;
end = 0;
- while (!cancel_upload && (blen = multipart_buffer_read(mbuff, buff, sizeof(buff), &end TSRMLS_CC)))
+
+ if (!cancel_upload) {
+ /* only bother to open temp file if we have data */
+ blen = multipart_buffer_read(mbuff, buff, sizeof(buff), &end TSRMLS_CC);
+#if DEBUG_FILE_UPLOAD
+ if (blen > 0) {
+#else
+ /* in non-debug mode we have no problem with 0-length files */
+ {
+#endif
+ fd = php_open_temporary_fd_ex(PG(upload_tmp_dir), "php", &temp_filename, 1 TSRMLS_CC);
+ upload_cnt--;
+ if (fd == -1) {
+ sapi_module.sapi_error(E_WARNING, "File upload error - unable to create a temporary file");
+ cancel_upload = UPLOAD_ERROR_E;
+ }
+ }
+ }
+
+ while (!cancel_upload && (blen > 0))
{
if (php_rfc1867_callback != NULL) {
multipart_event_file_data event_file_data;
@@ -1103,10 +1107,15 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
}
offset += wlen;
}
+
+ /* read data for next iteration */
+ blen = multipart_buffer_read(mbuff, buff, sizeof(buff), &end TSRMLS_CC);
}
+
if (fd != -1) { /* may not be initialized if file could not be created */
close(fd);
}
+
if (!cancel_upload && !end) {
#if DEBUG_FILE_UPLOAD
sapi_module.sapi_error(E_NOTICE, "Missing mime boundary at the end of the data for file %s", strlen(filename) > 0 ? filename : "");
View
101 tests/basic/rfc1867_max_file_uploads_empty_files.phpt
@@ -0,0 +1,101 @@
+--TEST--
+rfc1867 max_file_uploads - empty files shouldn't count (non-debug version)
+--SKIPIF--
+<?php if(function_exists("leak")) print "skip only for non-debug builds"; ?>
+--INI--
+file_uploads=1
+error_reporting=E_ALL
+max_file_uploads=2
+--POST_RAW--
+Content-Type: multipart/form-data; boundary=---------------------------20896060251896012921717172737
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file2"; filename=""
+Content-Type: text/plain-file
+
+
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file3"; filename=""
+Content-Type: text/plain-file
+
+33
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file4"; filename="file4.txt"
+Content-Type: text/plain-file
+
+
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file1"; filename="file1.txt"
+Content-Type: text/plain-file
+
+1
+-----------------------------20896060251896012921717172737--
+--FILE--
+<?php
+var_dump($_FILES);
+var_dump($_POST);
+if (is_uploaded_file($_FILES["file1"]["tmp_name"])) {
+ var_dump(file_get_contents($_FILES["file1"]["tmp_name"]));
+}
+if (is_uploaded_file($_FILES["file4"]["tmp_name"])) {
+ var_dump(file_get_contents($_FILES["file4"]["tmp_name"]));
+}
+?>
+--EXPECTF--
+array(4) {
+ ["file2"]=>
+ array(5) {
+ ["name"]=>
+ string(0) ""
+ ["type"]=>
+ string(0) ""
+ ["tmp_name"]=>
+ string(0) ""
+ ["error"]=>
+ int(4)
+ ["size"]=>
+ int(0)
+ }
+ ["file3"]=>
+ array(5) {
+ ["name"]=>
+ string(0) ""
+ ["type"]=>
+ string(0) ""
+ ["tmp_name"]=>
+ string(0) ""
+ ["error"]=>
+ int(4)
+ ["size"]=>
+ int(0)
+ }
+ ["file4"]=>
+ array(5) {
+ ["name"]=>
+ string(9) "file4.txt"
+ ["type"]=>
+ string(15) "text/plain-file"
+ ["tmp_name"]=>
+ string(%d) "%s"
+ ["error"]=>
+ int(0)
+ ["size"]=>
+ int(0)
+ }
+ ["file1"]=>
+ array(5) {
+ ["name"]=>
+ string(9) "file1.txt"
+ ["type"]=>
+ string(15) "text/plain-file"
+ ["tmp_name"]=>
+ string(%d) "%s"
+ ["error"]=>
+ int(0)
+ ["size"]=>
+ int(1)
+ }
+}
+array(0) {
+}
+string(1) "1"
+string(0) ""
View
102 tests/basic/rfc1867_max_file_uploads_empty_files_debug.phpt
@@ -0,0 +1,102 @@
+--TEST--
+rfc1867 max_file_uploads - empty files shouldn't count (debug version)
+--SKIPIF--
+<?php if(!function_exists("leak")) print "skip only for debug builds"; ?>
+--INI--
+file_uploads=1
+error_reporting=E_ALL
+max_file_uploads=1
+--POST_RAW--
+Content-Type: multipart/form-data; boundary=---------------------------20896060251896012921717172737
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file2"; filename=""
+Content-Type: text/plain-file
+
+
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file3"; filename=""
+Content-Type: text/plain-file
+
+33
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file4"; filename="file4.txt"
+Content-Type: text/plain-file
+
+
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file1"; filename="file1.txt"
+Content-Type: text/plain-file
+
+1
+-----------------------------20896060251896012921717172737--
+--FILE--
+<?php
+var_dump($_FILES);
+var_dump($_POST);
+if (is_uploaded_file($_FILES["file1"]["tmp_name"])) {
+ var_dump(file_get_contents($_FILES["file1"]["tmp_name"]));
+}
+?>
+--EXPECTF--
+Notice: No file uploaded in Unknown on line 0
+
+Notice: No file uploaded in Unknown on line 0
+
+Warning: Uploaded file size 0 - file [file4=file4.txt] not saved in Unknown on line 0
+array(4) {
+ ["file2"]=>
+ array(5) {
+ ["name"]=>
+ string(0) ""
+ ["type"]=>
+ string(0) ""
+ ["tmp_name"]=>
+ string(0) ""
+ ["error"]=>
+ int(4)
+ ["size"]=>
+ int(0)
+ }
+ ["file3"]=>
+ array(5) {
+ ["name"]=>
+ string(0) ""
+ ["type"]=>
+ string(0) ""
+ ["tmp_name"]=>
+ string(0) ""
+ ["error"]=>
+ int(4)
+ ["size"]=>
+ int(0)
+ }
+ ["file4"]=>
+ array(5) {
+ ["name"]=>
+ string(9) "file4.txt"
+ ["type"]=>
+ string(0) ""
+ ["tmp_name"]=>
+ string(0) ""
+ ["error"]=>
+ int(5)
+ ["size"]=>
+ int(0)
+ }
+ ["file1"]=>
+ array(5) {
+ ["name"]=>
+ string(9) "file1.txt"
+ ["type"]=>
+ string(15) "text/plain-file"
+ ["tmp_name"]=>
+ string(%d) "%s"
+ ["error"]=>
+ int(0)
+ ["size"]=>
+ int(1)
+ }
+}
+array(0) {
+}
+string(1) "1"

0 comments on commit 817c28c

Please sign in to comment.
Something went wrong with that request. Please try again.