- code cleanups

- cleanup progress data from session vars as soon as all post data has been readden (upload_progress.cleanup ini setting allows to disable this)
2025-01-19 18:24:15 +08:00 · 2010-05-11 16:39:07 +00:00 · 2010-05-11 16:39:07 +00:00 · 4f3b619625
commit 4f3b619625
parent d352196bd6
15 changed files with 278 additions and 27 deletions
--- a/ext/session/php_session.h
+++ b/ext/session/php_session.h
@ -166,6 +166,7 @@ typedef struct _php_ps_globals {

 	php_session_rfc1867_progress *rfc1867_progress;
 	zend_bool rfc1867_enabled; /* session.upload_progress.enabled */
+	zend_bool rfc1867_cleanup; /* session.upload_progress.cleanup */
 	smart_str rfc1867_prefix;  /* session.upload_progress.prefix */
 	smart_str rfc1867_name;    /* session.upload_progress.name */
 	long rfc1867_freq;         /* session.upload_progress.freq */
--- a/ext/session/session.c
+++ b/ext/session/session.c
@ -707,6 +707,8 @@ PHP_INI_BEGIN()
 	/* Upload progress */
 	STD_PHP_INI_BOOLEAN("session.upload_progress.enabled",
 	                                                "1",     ZEND_INI_PERDIR, OnUpdateBool,        rfc1867_enabled, php_ps_globals, ps_globals)
+	STD_PHP_INI_BOOLEAN("session.upload_progress.cleanup",
+	                                                "1",     ZEND_INI_PERDIR, OnUpdateBool,        rfc1867_cleanup, php_ps_globals, ps_globals)
 	STD_PHP_INI_ENTRY("session.upload_progress.prefix",
 	                                     "upload_progress_", ZEND_INI_PERDIR, OnUpdateSmartStr,      rfc1867_prefix,  php_ps_globals, ps_globals)
 	STD_PHP_INI_ENTRY("session.upload_progress.name",
@ -2057,17 +2059,30 @@ static const zend_module_dep session_deps[] = { /* {{{ */
   * Upload hook handling *
   ************************ */

-static void php_session_rfc1867_early_find_sid(php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
+static zend_bool early_find_sid_in(zval *dest, int where, php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
 {
 	zval **ppid;

+	if (!PG(http_globals)[where]) {
+		return 0;
+	}
+
+	if (zend_hash_find(Z_ARRVAL_P(PG(http_globals)[where]), PS(session_name), progress->sname_len+1, (void **)&ppid) == SUCCESS
+			&& Z_TYPE_PP(ppid) == IS_STRING) {
+		zval_dtor(dest);
+		ZVAL_ZVAL(dest, *ppid, 1, 0);
+		return 1;
+	}
+
+	return 0;
+} /* }}} */
+
+static void php_session_rfc1867_early_find_sid(php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
+{
+
 	if (PS(use_cookies)) {
 		sapi_module.treat_data(PARSE_COOKIE, NULL, NULL TSRMLS_CC);
-		if (PG(http_globals)[TRACK_VARS_COOKIE]
-			&& zend_hash_find(Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_COOKIE]), PS(session_name), progress->sname_len+1, (void **)&ppid) == SUCCESS) {
-			zval_dtor(&progress->sid);
-			ZVAL_ZVAL(&progress->sid, *ppid, 1, 0);
-			convert_to_string(&progress->sid);
+		if (early_find_sid_in(&progress->sid, TRACK_VARS_COOKIE, progress TSRMLS_CC)) {
 			progress->apply_trans_sid = 0;
 			return;
 		}
@ -2076,16 +2091,10 @@ static void php_session_rfc1867_early_find_sid(php_session_rfc1867_progress *pro
 		return;
 	}
 	sapi_module.treat_data(PARSE_GET, NULL, NULL TSRMLS_CC);
-	if (PG(http_globals)[TRACK_VARS_GET]
-		&& zend_hash_find(Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_GET]), PS(session_name), progress->sname_len+1, (void **)&ppid) == SUCCESS) {
-
-		zval_dtor(&progress->sid);
-		ZVAL_ZVAL(&progress->sid, *ppid, 1, 0);
-		convert_to_string(&progress->sid);
-	}
+	early_find_sid_in(&progress->sid, TRACK_VARS_GET, progress TSRMLS_CC);
 } /* }}} */

-static zend_bool php_session_rfc1867_check_cancel_upload(php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
+static zend_bool php_check_cancel_upload(php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
 {
 	zval **progress_ary, **cancel_upload;

@ -2098,8 +2107,7 @@ static zend_bool php_session_rfc1867_check_cancel_upload(php_session_rfc1867_pro
 	if (zend_hash_find(Z_ARRVAL_PP(progress_ary), "cancel_upload", sizeof("cancel_upload"), (void**)&cancel_upload) != SUCCESS) {
 		return 0;
 	}
-	return zend_is_true(*cancel_upload);
-
+	return Z_TYPE_PP(cancel_upload) == IS_BOOL && Z_LVAL_PP(cancel_upload);
 } /* }}} */

 static void php_session_rfc1867_update(php_session_rfc1867_progress *progress, int force_update TSRMLS_DC) /* {{{ */
@ -2126,14 +2134,22 @@ static void php_session_rfc1867_update(php_session_rfc1867_progress *progress, i
 	php_session_initialize(TSRMLS_C);
 	PS(session_status) = php_session_active;
 	IF_SESSION_VARS() {
-		if (!progress->cancel_upload && php_session_rfc1867_check_cancel_upload(progress TSRMLS_CC)) {
-			progress->cancel_upload = 1;
-		}
+		progress->cancel_upload = php_check_cancel_upload(progress TSRMLS_CC);
 		ZEND_SET_SYMBOL_WITH_LENGTH(Z_ARRVAL_P(PS(http_session_vars)), progress->key.c, progress->key.len+1, progress->data, 2, 0);
 	}
 	php_session_flush(TSRMLS_C);
 } /* }}} */

+static void php_session_rfc1867_cleanup(php_session_rfc1867_progress *progress TSRMLS_DC) /* {{{ */
+{
+	php_session_initialize(TSRMLS_C);
+	PS(session_status) = php_session_active;
+	IF_SESSION_VARS() {
+		zend_hash_del(Z_ARRVAL_P(PS(http_session_vars)), progress->key.c, progress->key.len+1);
+	}
+	php_session_flush(TSRMLS_C);
+} /* }}} */
+
 static int php_session_rfc1867_callback(unsigned int event, void *event_data, void **extra TSRMLS_DC) /* {{{ */
 {
 	php_session_rfc1867_progress *progress;
@ -2161,6 +2177,10 @@ static int php_session_rfc1867_callback(unsigned int event, void *event_data, vo
 			multipart_event_formdata *data = (multipart_event_formdata *) event_data;
 			size_t value_len;

+			if (Z_TYPE(progress->sid) && progress->key.c) {
+				break;
+			}
+
 			/* orig callback may have modified *data->newlength */
 			if (data->newlength) {
 				value_len = *data->newlength;
@ -2174,7 +2194,6 @@ static int php_session_rfc1867_callback(unsigned int event, void *event_data, vo
 				if (name_len == progress->sname_len && memcmp(data->name, PS(session_name), name_len) == 0) {
 					zval_dtor(&progress->sid);
 					ZVAL_STRINGL(&progress->sid, (*data->value), value_len, 1);
-					convert_to_string(&progress->sid);

 				} else if (name_len == PS(rfc1867_name).len && memcmp(data->name, PS(rfc1867_name).c, name_len) == 0) {
 					smart_str_free(&progress->key);
@ -2200,10 +2219,10 @@ static int php_session_rfc1867_callback(unsigned int event, void *event_data, vo
 			/* First FILE_START event, initializing data */
 			if (!progress->data) {

-				if (PS(rfc1867_freq) == 0) {
-					progress->update_step = 0;
+				if (PS(rfc1867_freq) >= 0) {
+					progress->update_step = PS(rfc1867_freq);
 				} else if (PS(rfc1867_freq) < 0) { // % of total size
-					progress->update_step = progress->content_length * PS(rfc1867_freq) / 100;
+					progress->update_step = progress->content_length * -PS(rfc1867_freq) / 100;
 				}
 				progress->next_update = 0;
 				progress->next_update_time = 0.0;
@ -2241,7 +2260,7 @@ static int php_session_rfc1867_callback(unsigned int event, void *event_data, vo
 			add_assoc_null_ex(progress->current_file, "tmp_name",        sizeof("tmp_name"));
 			add_assoc_long_ex(progress->current_file, "error",           sizeof("error"),           0);

-			add_assoc_long_ex(progress->current_file, "done",            sizeof("done"),            0);
+			add_assoc_bool_ex(progress->current_file, "done",            sizeof("done"),            0);
 			add_assoc_long_ex(progress->current_file, "start_time",      sizeof("start_time"),      (long)time(NULL));
 			add_assoc_zval_ex(progress->current_file, "bytes_processed", sizeof("bytes_processed"), progress->current_file_bytes_processed);

@ -2287,9 +2306,13 @@ static int php_session_rfc1867_callback(unsigned int event, void *event_data, vo
 			multipart_event_end *data = (multipart_event_end *) event_data;

 			if (Z_TYPE(progress->sid) && progress->key.c) {
-				add_assoc_bool_ex(progress->data, "done", sizeof("done"), 1);
-				Z_LVAL_P(progress->post_bytes_processed) = data->post_bytes_processed;
-				php_session_rfc1867_update(progress, 1 TSRMLS_CC);
+				if (PS(rfc1867_cleanup)) {
+					php_session_rfc1867_cleanup(progress TSRMLS_CC);
+				} else {
+					add_assoc_bool_ex(progress->data, "done", sizeof("done"), 1);
+					Z_LVAL_P(progress->post_bytes_processed) = data->post_bytes_processed;
+					php_session_rfc1867_update(progress, 1 TSRMLS_CC);
+				}
 				php_rshutdown_session_globals(TSRMLS_C);
 			}

--- a/ext/session/tests/rfc1867.phpt
+++ b/ext/session/tests/rfc1867.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=1%
--- a/ext/session/tests/rfc1867_cleanup.phpt
+++ b/ext/session/tests/rfc1867_cleanup.phpt
@ -0,0 +1,83 @@
+--TEST--
+session rfc1867
+--INI--
+file_uploads=1
+error_reporting=E_ALL&~E_NOTICE
+comment=debug builds show some additional E_NOTICE errors
+upload_max_filesize=1024
+session.save_path=
+session.name=PHPSESSID
+session.use_cookies=1
+session.use_only_cookies=0
+session.upload_progress.enabled=1
+session.upload_progress.cleanup=1
+session.upload_progress.prefix=upload_progress_
+session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
+session.upload_progress.freq=1%
+session.upload_progress.min_freq=0.000000001
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--COOKIE--
+PHPSESSID=rfc1867-tests
+--GET--
+PHPSESSID=rfc1867-tests-get
+--POST_RAW--
+Content-Type: multipart/form-data; boundary=---------------------------20896060251896012921717172737
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="PHPSESSID"
+
+rfc1867-tests-post
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="PHP_SESSION_UPLOAD_PROGRESS"
+
+rfc1867_cleanup.php
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file1"; filename="file1.txt"
+
+1
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file2"; filename="file2.txt"
+
+2
+-----------------------------20896060251896012921717172737--
+--FILE--
+<?php
+session_start();
+var_dump(session_id());
+var_dump(basename(__FILE__) == $_POST[ini_get("session.upload_progress.name")]);
+var_dump($_FILES);
+var_dump($_SESSION["upload_progress_" . basename(__FILE__)]);
+session_destroy();
+?>
+--EXPECTF--
+string(%d) "rfc1867-tests"
+bool(true)
+array(2) {
+  [%u|b%"file1"]=>
+  array(5) {
+    [%u|b%"name"]=>
+    %string|unicode%(9) "file1.txt"
+    [%u|b%"type"]=>
+    %string|unicode%(0) ""
+    [%u|b%"tmp_name"]=>
+    %string|unicode%(%d) "%s"
+    [%u|b%"error"]=>
+    int(0)
+    [%u|b%"size"]=>
+    int(1)
+  }
+  [%u|b%"file2"]=>
+  array(5) {
+    [%u|b%"name"]=>
+    %string|unicode%(9) "file2.txt"
+    [%u|b%"type"]=>
+    %string|unicode%(0) ""
+    [%u|b%"tmp_name"]=>
+    %string|unicode%(%d) "%s"
+    [%u|b%"error"]=>
+    int(0)
+    [%u|b%"size"]=>
+    int(1)
+  }
+}
+NULL
--- a/ext/session/tests/rfc1867_disabled.phpt
+++ b/ext/session/tests/rfc1867_disabled.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=0
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=1%
--- a/ext/session/tests/rfc1867_disabled_2.phpt
+++ b/ext/session/tests/rfc1867_disabled_2.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=1%
--- a/ext/session/tests/rfc1867_inter.phpt
+++ b/ext/session/tests/rfc1867_inter.phpt
@ -0,0 +1,133 @@
+--TEST--
+session rfc1867
+--INI--
+file_uploads=1
+error_reporting=E_ALL&~E_NOTICE
+comment=debug builds show some additional E_NOTICE errors
+upload_max_filesize=1024
+session.save_path=
+session.name=PHPSESSID
+session.use_cookies=1
+session.use_only_cookies=0
+session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
+session.upload_progress.prefix=upload_progress_
+session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
+session.upload_progress.freq=1%
+session.upload_progress.min_freq=0.000000001
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--COOKIE--
+PHPSESSID=rfc1867-tests
+--GET--
+PHPSESSID=rfc1867-tests-get
+--POST_RAW--
+Content-Type: multipart/form-data; boundary=---------------------------20896060251896012921717172737
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="PHPSESSID"
+
+rfc1867-tests-post
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="PHP_SESSION_UPLOAD_PROGRESS"
+
+rfc1867_inter.php_1
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file1"; filename="file1.txt"
+
+1
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="PHP_SESSION_UPLOAD_PROGRESS"
+
+rfc1867_inter.php_2
+-----------------------------20896060251896012921717172737
+Content-Disposition: form-data; name="file2"; filename="file2.txt"
+
+2
+-----------------------------20896060251896012921717172737--
+--FILE--
+<?php
+session_start();
+var_dump(session_id());
+var_dump($_FILES);
+var_dump($_SESSION["upload_progress_" . basename(__FILE__) . "_1"]);
+var_dump($_SESSION["upload_progress_" . basename(__FILE__) . "_2"]);
+session_destroy();
+?>
+--EXPECTF--
+string(%d) "rfc1867-tests"
+array(2) {
+  [%u|b%"file1"]=>
+  array(5) {
+    [%u|b%"name"]=>
+    %string|unicode%(9) "file1.txt"
+    [%u|b%"type"]=>
+    %string|unicode%(0) ""
+    [%u|b%"tmp_name"]=>
+    %string|unicode%(%d) "%s"
+    [%u|b%"error"]=>
+    int(0)
+    [%u|b%"size"]=>
+    int(1)
+  }
+  [%u|b%"file2"]=>
+  array(5) {
+    [%u|b%"name"]=>
+    %string|unicode%(9) "file2.txt"
+    [%u|b%"type"]=>
+    %string|unicode%(0) ""
+    [%u|b%"tmp_name"]=>
+    %string|unicode%(%d) "%s"
+    [%u|b%"error"]=>
+    int(0)
+    [%u|b%"size"]=>
+    int(1)
+  }
+}
+array(5) {
+  [%u|b%"start_time"]=>
+  int(%d)
+  [%u|b%"content_length"]=>
+  int(%d)
+  [%u|b%"bytes_processed"]=>
+  int(%d)
+  [%u|b%"done"]=>
+  bool(true)
+  [%u|b%"files"]=>
+  array(2) {
+    [0]=>
+    array(7) {
+      [%u|b%"field_name"]=>
+      %unicode|string%(5) "file1"
+      [%u|b%"name"]=>
+      %unicode|string%(9) "file1.txt"
+      [%u|b%"tmp_name"]=>
+      %unicode|string%(%d) "%s"
+      [%u|b%"error"]=>
+      int(0)
+      [%u|b%"done"]=>
+      bool(true)
+      [%u|b%"start_time"]=>
+      int(%d)
+      [%u|b%"bytes_processed"]=>
+      int(1)
+    }
+    [1]=>
+    array(7) {
+      [%u|b%"field_name"]=>
+      %unicode|string%(5) "file2"
+      [%u|b%"name"]=>
+      %unicode|string%(9) "file2.txt"
+      [%u|b%"tmp_name"]=>
+      %unicode|string%(%d) "%s"
+      [%u|b%"error"]=>
+      int(0)
+      [%u|b%"done"]=>
+      bool(true)
+      [%u|b%"start_time"]=>
+      int(%d)
+      [%u|b%"bytes_processed"]=>
+      int(1)
+    }
+  }
+}
+NULL
--- a/ext/session/tests/rfc1867_no_name.phpt
+++ b/ext/session/tests/rfc1867_no_name.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=1%
--- a/ext/session/tests/rfc1867_sid_cookie.phpt
+++ b/ext/session/tests/rfc1867_sid_cookie.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_get.phpt
+++ b/ext/session/tests/rfc1867_sid_get.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_get_2.phpt
+++ b/ext/session/tests/rfc1867_sid_get_2.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=0
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_invalid.phpt
+++ b/ext/session/tests/rfc1867_sid_invalid.phpt
@ -11,6 +11,7 @@ session.use_cookies=1
 session.use_only_cookies=0
 session.auto_start=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_only_cookie.phpt
+++ b/ext/session/tests/rfc1867_sid_only_cookie.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=1
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_only_cookie_2.phpt
+++ b/ext/session/tests/rfc1867_sid_only_cookie_2.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=1
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0
--- a/ext/session/tests/rfc1867_sid_post.phpt
+++ b/ext/session/tests/rfc1867_sid_post.phpt
@ -10,6 +10,7 @@ session.name=PHPSESSID
 session.use_cookies=1
 session.use_only_cookies=0
 session.upload_progress.enabled=1
+session.upload_progress.cleanup=0
 session.upload_progress.prefix=upload_progress_
 session.upload_progress.name=PHP_SESSION_UPLOAD_PROGRESS
 session.upload_progress.freq=0