Merge pull request #2705 from Kenpachi2k13/string_var_size

Cap string variable value length and unify corresponding SQL columns size

16 files changed, 120 insertions, 40 deletions

diff --git a/doc/script_commands.txt b/doc/script_commands.txt
index 3b77aeb2c..3c0b37a85 100644
--- a/doc/script_commands.txt
+++ b/doc/script_commands.txt
@@ -536,7 +536,9 @@ variables or an empty string ("", nothing between the quotes) for string
 variables. Once you set it to that, the variable is as good as forgotten
 forever, and no trace remains of it even if it was stored with character
 or account data. The maximum length of variable name including prefix and
-suffix is 32.
+suffix is 32. Permanent string variables (name$, $name$, #name$, ##name$)
+can store text with a maximum length of 255 characters. All other string
+type variables have no such limitation.
 
 Some variables are special, that is, they are already defined for you by
 the scripting engine. You can see the full list somewhere in
@@ -1710,7 +1712,8 @@ The default value of 'min' and 'max' can be set with 'input_min_value' and
 For numeric inputs the value is capped to the range [min, max]. Returns 1
 if the value was higher than 'max', -1 if lower than 'min' and 0 otherwise.
 For string inputs it returns 1 if the string was longer than 'max', -1 is
-shorter than 'min' and 0 otherwise.
+shorter than 'min' and 0 otherwise. Note that an input string has a maximum
+length of 70 characters.
 
 ---------------------------------------
 
diff --git a/sql-files/main.sql b/sql-files/main.sql
index dcd7e543d..d5b7735fc 100644
--- a/sql-files/main.sql
+++ b/sql-files/main.sql
@@ -51,7 +51,7 @@ CREATE TABLE IF NOT EXISTS `acc_reg_str_db` (
   `account_id` INT UNSIGNED NOT NULL DEFAULT '0',
   `key` VARCHAR(32) BINARY NOT NULL DEFAULT '',
   `index` INT UNSIGNED NOT NULL DEFAULT '0',
-  `value` VARCHAR(254) NOT NULL DEFAULT '0',
+  `value` VARCHAR(255) NOT NULL DEFAULT '0',
   PRIMARY KEY (`account_id`,`key`,`index`),
   KEY `account_id` (`account_id`)
 ) ENGINE=MyISAM;
@@ -275,7 +275,7 @@ CREATE TABLE IF NOT EXISTS `char_reg_str_db` (
   `char_id` INT UNSIGNED NOT NULL DEFAULT '0',
   `key` VARCHAR(32) BINARY NOT NULL DEFAULT '',
   `index` INT UNSIGNED NOT NULL DEFAULT '0',
-  `value` VARCHAR(254) NOT NULL DEFAULT '0',
+  `value` VARCHAR(255) NOT NULL DEFAULT '0',
   PRIMARY KEY (`char_id`,`key`,`index`),
   KEY `char_id` (`char_id`)
 ) ENGINE=MyISAM;
@@ -372,7 +372,7 @@ CREATE TABLE IF NOT EXISTS `global_acc_reg_str_db` (
   `account_id` INT UNSIGNED NOT NULL DEFAULT '0',
   `key` VARCHAR(32) BINARY NOT NULL DEFAULT '',
   `index` INT UNSIGNED NOT NULL DEFAULT '0',
-  `value` VARCHAR(254) NOT NULL DEFAULT '0',
+  `value` VARCHAR(255) NOT NULL DEFAULT '0',
   PRIMARY KEY (`account_id`,`key`,`index`),
   KEY `account_id` (`account_id`)
 ) ENGINE=MyISAM;
@@ -938,6 +938,7 @@ INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1574463539); -- 2019-11-2
 INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1579817630); -- 2020-01-24--01-09.sql
 INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1584838560); -- 2020-03-22--01-56.sql
 INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1584842940); -- 2020-03-22--03-09.sql
+INSERT IGNORE INTO `sql_updates` (`timestamp`) VALUES (1588301040); -- 2020-05-01--04-44.sql
 
 --
 -- Table structure for table `storage`
diff --git a/sql-files/upgrades/2020-05-01--04-44.sql b/sql-files/upgrades/2020-05-01--04-44.sql
new file mode 100644
index 000000000..6cb5a30ec
--- /dev/null
+++ b/sql-files/upgrades/2020-05-01--04-44.sql
@@ -0,0 +1,25 @@
+#1588301040
+
+-- This file is part of Hercules.
+-- http://herc.ws - http://github.com/HerculesWS/Hercules
+--
+-- Copyright (C) 2019-2020 Hercules Dev Team
+--
+-- Hercules is free software: you can redistribute it and/or modify
+-- it under the terms of the GNU General Public License as published by
+-- the Free Software Foundation, either version 3 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+-- GNU General Public License for more details.
+--
+-- You should have received a copy of the GNU General Public License
+-- along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+ALTER TABLE `acc_reg_str_db` MODIFY `value` VARCHAR(255) NOT NULL DEFAULT '0';
+ALTER TABLE `char_reg_str_db` MODIFY `value` VARCHAR(255) NOT NULL DEFAULT '0';
+ALTER TABLE `global_acc_reg_str_db` MODIFY `value` VARCHAR(255) NOT NULL DEFAULT '0';
+
+INSERT INTO `sql_updates` (`timestamp`) VALUES (1588301040);
diff --git a/sql-files/upgrades/index.txt b/sql-files/upgrades/index.txt
index fdd5fcc5a..64a7793f8 100644
--- a/sql-files/upgrades/index.txt
+++ b/sql-files/upgrades/index.txt
@@ -62,3 +62,4 @@
 2020-01-24--01-09.sql
 2020-03-22--01-56.sql
 2020-03-22--03-09.sql
+2020-05-01--04-44.sql
diff --git a/src/char/char.c b/src/char/char.c
index 0406dbecf..c61b6107a 100644
--- a/src/char/char.c
+++ b/src/char/char.c
@@ -2902,13 +2902,13 @@ static void char_global_accreg_to_login_add(const char *key, unsigned int index,
 
 		if( val ) {
 			char *sval = (char*)val;
-			len = strlen(sval)+1;
+			len = strlen(sval);
 
-			WFIFOB(chr->login_fd, nlen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+			WFIFOB(chr->login_fd, nlen) = (unsigned char)len; // Won't be higher; the column size is 255.
 			nlen += 1;
 
-			safestrncpy(WFIFOP(chr->login_fd,nlen), sval, len);
-			nlen += len;
+			safestrncpy(WFIFOP(chr->login_fd, nlen), sval, len + 1);
+			nlen += len + 1;
 		}
 	} else {
 		WFIFOB(chr->login_fd, nlen) = val ? 0 : 1;
diff --git a/src/char/inter.c b/src/char/inter.c
index 2d8d06a9c..5252b3315 100644
--- a/src/char/inter.c
+++ b/src/char/inter.c
@@ -676,13 +676,13 @@ static int inter_accreg_fromsql(int account_id, int char_id, int fd, int type)
 		plen += 4;
 
 		SQL->GetData(inter->sql_handle, 2, &data, NULL);
-		len = strlen(data)+1;
+		len = strlen(data);
 
-		WFIFOB(fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+		WFIFOB(fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 		plen += 1;
 
-		safestrncpy(WFIFOP(fd,plen), data, len);
-		plen += len;
+		safestrncpy(WFIFOP(fd, plen), data, len + 1);
+		plen += len + 1;
 
 		WFIFOW(fd, 14) += 1;
 
diff --git a/src/char/mapif.c b/src/char/mapif.c
index 9077afae4..f0c886586 100644
--- a/src/char/mapif.c
+++ b/src/char/mapif.c
@@ -2030,7 +2030,8 @@ static int mapif_parse_Registry(int fd)
 
 	if (count != 0) {
 		int cursor = 14, i;
-		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH + 1];
+		char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 		bool isLoginActive = sockt->session_is_active(chr->login_fd);
 
 		if (isLoginActive)
@@ -2057,8 +2058,8 @@ static int mapif_parse_Registry(int fd)
 			/* str */
 			case 2:
 				len = RFIFOB(fd, cursor);
-				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-				cursor += len + 1;
+				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+				cursor += len + 2;
 				inter->savereg(account_id, char_id, key, index, (intptr_t)sval, true);
 				break;
 			case 3:
diff --git a/src/common/mmo.h b/src/common/mmo.h
index 9421f6e35..d2f3aa8f1 100644
--- a/src/common/mmo.h
+++ b/src/common/mmo.h
@@ -380,7 +380,10 @@ STATIC_ASSERT(MAX_ITEM_OPTIONS <= 5, "This value is limited by the client and da
 #define JOBL_BABY  0x2000
 #define JOBL_THIRD 0x4000
 
-#define SCRIPT_VARNAME_LENGTH 32 ///< Maximum length of a script variable
+#define SCRIPT_VARNAME_LENGTH 32 ///< Maximum length of a script variable's name including affixes and excluding NULL-terminator.
+STATIC_ASSERT(SCRIPT_VARNAME_LENGTH <= 32, "This value is limited by the inter-server communication and database layout and should only be increased if you know the consequences.");
+#define SCRIPT_STRING_VAR_LENGTH 255 ///< Maximum length of strings stored in script variables excluding NULL-terminator.
+STATIC_ASSERT(SCRIPT_STRING_VAR_LENGTH <= 255, "This value is limited by the inter-server communication and database layout and should only be increased if you know the consequences.");
 
 #define INFINITE_DURATION (-1) // Infinite duration for status changes
 
diff --git a/src/login/account.c b/src/login/account.c
index 3632c257a..ec0bc81e8 100644
--- a/src/login/account.c
+++ b/src/login/account.c
@@ -632,7 +632,8 @@ static void account_mmo_save_accreg2(AccountDB *self, int fd, int account_id, in
 	sql_handle = db->accounts;
 	if (count) {
 		int cursor = 14, i;
-		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH + 1];
+		char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 
 		for (i = 0; i < count; i++) {
 			unsigned int index;
@@ -657,8 +658,8 @@ static void account_mmo_save_accreg2(AccountDB *self, int fd, int account_id, in
 				/* str */
 				case 2:
 					len = RFIFOB(fd, cursor);
-					safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-					cursor += len + 1;
+					safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+					cursor += len + 2;
 					if( SQL_ERROR == SQL->Query(sql_handle, "REPLACE INTO `%s` (`account_id`,`key`,`index`,`value`) VALUES ('%d','%s','%u','%s')", db->global_acc_reg_str_db, account_id, key, index, sval) )
 						Sql_ShowDebug(sql_handle);
 					break;
@@ -719,13 +720,13 @@ static void account_mmo_send_accreg2(AccountDB *self, int fd, int account_id, in
 		plen += 4;
 
 		SQL->GetData(sql_handle, 2, &data, NULL);
-		len = strlen(data)+1;
+		len = strlen(data);
 
-		WFIFOB(fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+		WFIFOB(fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 		plen += 1;
 
-		safestrncpy(WFIFOP(fd,plen), data, len);
-		plen += len;
+		safestrncpy(WFIFOP(fd, plen), data, len + 1);
+		plen += len + 1;
 
 		WFIFOW(fd, 14) += 1;
 
diff --git a/src/map/atcommand.c b/src/map/atcommand.c
index 3fe6c8581..54cc9e2c9 100644
--- a/src/map/atcommand.c
+++ b/src/map/atcommand.c
@@ -8916,13 +8916,17 @@ ACMD(accinfo)
 /* [Ind] */
 ACMD(set)
 {
-	char reg[SCRIPT_VARNAME_LENGTH+1], val[254];
+	char reg[SCRIPT_VARNAME_LENGTH + 1];
+	char val[SCRIPT_STRING_VAR_LENGTH + 1];
 	struct script_data* data;
 	int toset = 0;
 	bool is_str = false;
 	size_t len;
 
-	if (!*message || (toset = sscanf(message, "%32s %253[^\n]", reg, val)) < 1) {
+	char format[20];
+	safesnprintf(format, sizeof(format), "%%%ds %%%d[^\\n]", SCRIPT_VARNAME_LENGTH, SCRIPT_STRING_VAR_LENGTH);
+
+	if (*message == '\0' || (toset = sscanf(message, format, reg, val)) < 1) {
 		clif->message(fd, msg_fd(fd,1367)); // Usage: @set <variable name> <value>
 		clif->message(fd, msg_fd(fd,1368)); // Usage: ex. "@set PoringCharVar 50"
 		clif->message(fd, msg_fd(fd,1369)); // Usage: ex. "@set PoringCharVarSTR$ Super Duper String"
diff --git a/src/map/clif.c b/src/map/clif.c
index 7be5c6978..ab13ffe1f 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -11794,7 +11794,7 @@ static void clif_parse_WisMessage(int fd, struct map_session_data *sd)
 		char *str = target + 4; // Skip the NPC: string part.
 		struct npc_data *nd;
 		if ((nd = npc->name2id(str))) {
-			char split_data[NUM_WHISPER_VAR][CHAT_SIZE_MAX];
+			char split_data[NUM_WHISPER_VAR][SCRIPT_STRING_VAR_LENGTH + 1];
 			char *split;
 			char output[256];
 
diff --git a/src/map/intif.c b/src/map/intif.c
index a420b7204..276b40780 100644
--- a/src/map/intif.c
+++ b/src/map/intif.c
@@ -218,13 +218,13 @@ static int intif_saveregistry(struct map_session_data *sd)
 			plen += 1;
 
 			if( p->value ) {
-				len = strlen(p->value)+1;
+				len = strlen(p->value);
 
-				WFIFOB(inter_fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+				WFIFOB(inter_fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 				plen += 1;
 
-				safestrncpy(WFIFOP(inter_fd,plen), p->value, len);
-				plen += len;
+				safestrncpy(WFIFOP(inter_fd, plen), p->value, len + 1);
+				plen += len + 1;
 			} else {
 				script->reg_destroy_single(sd,key.i64,&p->flag);
 			}
@@ -1025,7 +1025,7 @@ static void intif_parse_Registers(int fd)
 		 * { keyLength(B), key(<keyLength>), index(L), valLength(B), val(<valLength>) }
 		 **/
 		if (type) {
-			char sval[254];
+			char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 			for (i = 0; i < max; i++) {
 				int len = RFIFOB(fd, cursor);
 				safestrncpy(key, RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
@@ -1035,8 +1035,8 @@ static void intif_parse_Registers(int fd)
 				cursor += 4;
 
 				len = RFIFOB(fd, cursor);
-				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-				cursor += len + 1;
+				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+				cursor += len + 2;
 
 				script->set_reg(NULL,sd,reference_uid(script->add_variable(key), index), key, sval, NULL);
 			}
diff --git a/src/map/mapreg_sql.c b/src/map/mapreg_sql.c
index 741505e17..2963637da 100644
--- a/src/map/mapreg_sql.c
+++ b/src/map/mapreg_sql.c
@@ -176,9 +176,9 @@ static bool mapreg_setregstr(int64 uid, const char *str)
 
 			if(name[1] != '@' && !mapreg->skip_insert) { //put returned null, so we must insert.
 				char tmp_str[(SCRIPT_VARNAME_LENGTH+1)*2+1];
-				char tmp_str2[255*2+1];
+				char tmp_str2[SCRIPT_STRING_VAR_LENGTH * 2 + 1];
 				SQL->EscapeStringLen(map->mysql_handle, tmp_str, name, strnlen(name, SCRIPT_VARNAME_LENGTH+1));
-				SQL->EscapeStringLen(map->mysql_handle, tmp_str2, str, strnlen(str, 255));
+				SQL->EscapeStringLen(map->mysql_handle, tmp_str2, str, strnlen(str, SCRIPT_STRING_VAR_LENGTH));
 				if( SQL_ERROR == SQL->Query(map->mysql_handle, "INSERT INTO `%s`(`varname`,`index`,`value`) VALUES ('%s','%u','%s')", mapreg->table, tmp_str, i, tmp_str2) )
 					Sql_ShowDebug(map->mysql_handle);
 			}
@@ -203,7 +203,7 @@ static void script_load_mapreg(void)
 	struct SqlStmt *stmt = SQL->StmtMalloc(map->mysql_handle);
 	char varname[SCRIPT_VARNAME_LENGTH+1];
 	int index;
-	char value[255+1];
+	char value[SCRIPT_STRING_VAR_LENGTH + 1];
 	uint32 length;
 
 	if ( SQL_ERROR == SQL->StmtPrepare(stmt, "SELECT `varname`, `index`, `value` FROM `%s`", mapreg->table)
@@ -261,8 +261,8 @@ static void script_save_mapreg(void)
 					if( SQL_ERROR == SQL->Query(map->mysql_handle, "UPDATE `%s` SET `value`='%d' WHERE `varname`='%s' AND `index`='%d' LIMIT 1", mapreg->table, m->u.i, name, i) )
 						Sql_ShowDebug(map->mysql_handle);
 				} else {
-					char tmp_str2[2*255+1];
-					SQL->EscapeStringLen(map->mysql_handle, tmp_str2, m->u.str, safestrnlen(m->u.str, 255));
+					char tmp_str2[SCRIPT_STRING_VAR_LENGTH * 2 + 1];
+					SQL->EscapeStringLen(map->mysql_handle, tmp_str2, m->u.str, safestrnlen(m->u.str, SCRIPT_STRING_VAR_LENGTH));
 					if( SQL_ERROR == SQL->Query(map->mysql_handle, "UPDATE `%s` SET `value`='%s' WHERE `varname`='%s' AND `index`='%d' LIMIT 1", mapreg->table, tmp_str2, name, i) )
 						Sql_ShowDebug(map->mysql_handle);
 				}
diff --git a/src/map/npc_chat.c b/src/map/npc_chat.c
index 0ca84cff4..0df323e96 100644
--- a/src/map/npc_chat.c
+++ b/src/map/npc_chat.c
@@ -394,7 +394,8 @@ static int npc_chat_sub(struct block_list *bl, va_list ap)
 				// save out the matched strings
 				for (i = 0; i < r; i++)
 				{
-					char var[15], val[255];
+					char var[SCRIPT_VARNAME_LENGTH + 1];
+					char val[SCRIPT_STRING_VAR_LENGTH + 1];
 					snprintf(var, sizeof(var), "$@p%i$", i);
 					libpcre->copy_substring(msg, offsets, r, i, val, sizeof(val));
 					script->set_var(sd, var, val);
diff --git a/src/map/script.c b/src/map/script.c
index 45c954dc0..347ef3d2a 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -3395,6 +3395,32 @@ static void set_reg_instance_num(struct script_state *st, int64 num, const char
 }
 
 /**
+ * Validates if a variable is permanent (stored in database) by passed variable name.
+ *
+ * @param name The variable name to validate.
+ * @return True if variable is permanent, otherwise false.
+ *
+ **/
+static bool script_is_permanent_variable(const char *name)
+{
+	nullpo_retr(false, name);
+
+	if (strlen(name) == 0)
+		return false;
+
+	if (ISALNUM(name[0]) != 0)
+		return true; // Permanent characater variable.
+
+	if (name[0] == '#')
+		return true; // Permanent (global) account variable.
+
+	if (strlen(name) > 1 && name[0] == '$' && ISALNUM(name[1]) != 0)
+		return true; // Permanent server variable.
+
+	return false;
+}
+
+/**
  * Stores the value of a script variable
  *
  * @param st    current script state.
@@ -3439,6 +3465,18 @@ static int set_reg(struct script_state *st, struct map_session_data *sd, int64 n
 	if (is_string_variable(name)) {// string variable
 		const char *str = (const char*)value;
 
+		if (script->is_permanent_variable(name) && strlen(str) > SCRIPT_STRING_VAR_LENGTH) {
+			ShowError("script:set_reg: Value of variable %s is too long: %lu! Maximum is %d. Skipping...\n",
+				  name, strlen(str), SCRIPT_STRING_VAR_LENGTH);
+
+			if (st != NULL) {
+				script->reportsrc(st);
+				st->state = END;
+			}
+
+			return 0;
+		}
+
 		switch (prefix) {
 		case '@':
 			if (ref) {
@@ -28255,6 +28293,7 @@ void script_defaults(void)
 	script->load_parameters = script_load_parameters;
 	script->print_line = script_print_line;
 	script->errorwarning_sub = script_errorwarning_sub;
+	script->is_permanent_variable = script_is_permanent_variable;
 	script->set_reg = set_reg;
 	script->set_reg_ref_str = set_reg_npcscope_str;
 	script->set_reg_pc_ref_str = set_reg_pc_ref_str;
diff --git a/src/map/script.h b/src/map/script.h
index 511497a66..5fa81dc0e 100644
--- a/src/map/script.h
+++ b/src/map/script.h
@@ -984,6 +984,7 @@ struct script_interface {
 	void (*load_parameters) (void);
 	const char* (*print_line) (StringBuf *buf, const char *p, const char *mark, int line);
 	void (*errorwarning_sub) (StringBuf *buf, const char *src, const char *file, int start_line, const char *error_msg, const char *error_pos);
+	bool (*is_permanent_variable) (const char *name);
 	int (*set_reg) (struct script_state *st, struct map_session_data *sd, int64 num, const char *name, const void *value, struct reg_db *ref);
 	void (*set_reg_ref_str) (struct script_state* st, struct reg_db *n, int64 num, const char* name, const char *str);
 	void (*set_reg_pc_ref_str) (struct script_state* st, struct reg_db *n, int64 num, const char* name, const char *str);