<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40196 >
On 07/04/2008, Pepeto _ wrote:
>
> Try with this patch.
Bugs I reported cannot be reproduced when this patch is applied.
Still, I don't think patch is correct. At the very least changes you
make should not be required (if other parts of the code work
correctly), and I'm not entirely sure that they themselves never act
as bug.
Note that if *str0 != '\0' (so it will pass the test) and *str1 ==
'\0' (which you are testing), they are not equal (later test).
> I also noticed a long time ago that the client sends lot of wrong
> commands to the server, confusing player names and connection names, the
> commands names and adding '"' characters that the server doesn't remove
> when it interpret it. See PR#39614.
Actually, it's not client bug that it sends quoted command
parameters. It's server bug that they are not handled correctly.
Inspecting all these issues indeed end to same core bug; length is not
handled correctly for quoted string. match_prefix() considers quoted
and non-quoted strings different because their length, and sometimes
does not compare enough characters: (Comparing """""" and cazfi are
sometimes considered exact match as there's five characters to match
in cazfi, and five first characters of """"" are to be ignored)
Instead of adding minimal fix (which would be ugly hack) to existing
system, attached patch makes soem overall changes to quoted string
matching. To me, this new behavior seems much more sensible than old
one.
- Only strings both starting and ending with '"' are considered
quoted and these starting and ending quotes are only ones ignored
during matching
- Correct string length comparison for quoted strings implemented and used
Remaining minor issue is that server messages display parameters as
they were originally given.
/aitoggle cazfi -> /aitoggle: No player by the name of 'cazfi'.
/aitoggle "cazfi" -> /aitoggle: No player by the name of '"cazfi"'.
- ML
diff -Nurd -X.diff_ignore freeciv/common/connection.c freeciv/common/connection.c
--- freeciv/common/connection.c 2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/connection.c 2008-04-08 17:21:10.000000000 +0300
@@ -440,7 +440,8 @@
*result = match_prefix(connection_accessor,
conn_list_size(game.all_connections),
- MAX_LEN_NAME-1, mystrncasecmp, user_name, &ind);
+ MAX_LEN_NAME-1, mystrncasecmp, NULL,
+ user_name, &ind);
if (*result < M_PRE_AMBIGUOUS) {
return conn_list_get(game.all_connections, ind);
diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c 2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/player.c 2008-04-08 17:22:42.000000000 +0300
@@ -396,7 +396,8 @@
int ind;
*result = match_prefix(player_name_by_number, game.info.nplayers,
- MAX_LEN_NAME-1, mystrncasequotecmp, name, &ind);
+ MAX_LEN_NAME-1, mystrncasequotecmp,
+ effectivestrlenquote, name, &ind);
if (*result < M_PRE_AMBIGUOUS) {
return player_by_number(ind);
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c 2008-04-06 11:24:53.000000000 +0300
+++ freeciv/server/stdinhand.c 2008-04-08 17:26:51.000000000 +0300
@@ -173,7 +173,7 @@
int ind;
result = match_prefix(command_name_by_number, CMD_NUM, 0,
- mystrncasecmp, token, &ind);
+ mystrncasecmp, NULL, token, &ind);
if (result < M_PRE_AMBIGUOUS) {
return ind;
@@ -1486,7 +1486,7 @@
}
result = match_prefix(optname_accessor, SETTINGS_NUM, 0, mystrncasecmp,
- name, &ind);
+ NULL, name, &ind);
return ((result < M_PRE_AMBIGUOUS) ? ind :
(result == M_PRE_AMBIGUOUS) ? -2 : -1);
@@ -4008,7 +4008,7 @@
/* no commands means no help, either */
match_result = match_prefix(helparg_accessor, HELP_ARG_NUM, 0,
- mystrncasecmp, arg, &ind);
+ mystrncasecmp, NULL, arg, &ind);
if (match_result==M_PRE_EMPTY) {
show_help_intro(caller, CMD_HELP);
@@ -4082,7 +4082,7 @@
remove_leading_trailing_spaces(arg);
match_result = match_prefix(listarg_accessor, LIST_ARG_NUM, 0,
- mystrncasecmp, arg, &ind_int);
+ mystrncasecmp, NULL, arg, &ind_int);
ind = ind_int;
if (match_result > M_PRE_EMPTY) {
diff -Nurd -X.diff_ignore freeciv/utility/shared.c freeciv/utility/shared.c
--- freeciv/utility/shared.c 2008-03-11 11:27:15.000000000 +0200
+++ freeciv/utility/shared.c 2008-04-08 17:19:54.000000000 +0300
@@ -1485,12 +1485,17 @@
size_t n_names,
size_t max_len_name,
m_pre_strncmp_fn_t cmp_fn,
+ m_strlen_fn_t len_fn,
const char *prefix,
int *ind_result)
{
int i, len, nmatches;
- len = strlen(prefix);
+ if (len_fn == NULL) {
+ len = strlen(prefix);
+ } else {
+ len = len_fn(prefix);
+ }
if (len == 0) {
return M_PRE_EMPTY;
}
diff -Nurd -X.diff_ignore freeciv/utility/shared.h freeciv/utility/shared.h
--- freeciv/utility/shared.h 2008-03-08 16:32:11.000000000 +0200
+++ freeciv/utility/shared.h 2008-04-08 17:21:57.000000000 +0300
@@ -224,10 +224,14 @@
/* function type to compare prefix: */
typedef int (*m_pre_strncmp_fn_t)(const char *, const char *, size_t n);
+/* function type to calculate effective string length: */
+typedef size_t (m_strlen_fn_t)(const char *str);
+
enum m_pre_result match_prefix(m_pre_accessor_fn_t accessor_fn,
size_t n_names,
size_t max_len_name,
m_pre_strncmp_fn_t cmp_fn,
+ m_strlen_fn_t len_fn,
const char *prefix,
int *ind_result);
diff -Nurd -X.diff_ignore freeciv/utility/support.c freeciv/utility/support.c
--- freeciv/utility/support.c 2008-01-31 01:26:00.000000000 +0200
+++ freeciv/utility/support.c 2008-04-08 22:46:24.000000000 +0300
@@ -135,24 +135,66 @@
}
/***************************************************************
- Compare strings like strncasecmp() but ignoring quotes in
- either string. Quotes still count towards n.
+ Count length of string without possible surrounding quotes.
+***************************************************************/
+size_t effectivestrlenquote(const char *str)
+{
+ int len = strlen(str);
+
+ if (str[0] == '"' && str[len-1] == '"') {
+ return len - 2;
+ }
+
+ return len;
+}
+
+/***************************************************************
+ Compare strings like strncasecmp() but ignoring surrounding
+ quotes in either string.
***************************************************************/
int mystrncasequotecmp(const char *str0, const char *str1, size_t n)
{
size_t i;
- char c;
+ size_t len0 = strlen(str0); /* TODO: We iterate string once already here, */
+ size_t len1 = strlen(str1); /* could iterate only once */
+ size_t cmplen;
- for (i = 0; i < n && *str0 != '\0'; i++) {
- if (my_tolower(*str0) != my_tolower(*str1)
- && *str0 != '"' && *str1 != '"') {
+ if (str0[0] == '"') {
+ if (str0[len0 - 1] == '"') {
+ /* Surrounded with quotes */
+ str0++;
+ len0 -= 2;
+ }
+ }
+
+ if (str1[0] == '"') {
+ if (str1[len1 - 1] == '"') {
+ /* Surrounded with quotes */
+ str1++;
+ len1 -= 2;
+ }
+ }
+
+ if (len0 < n || len1 < n) {
+ /* One of the strings is shorter than what should be compared... */
+ if (len0 != len1) {
+ /* ...and another is longer than it. */
+ return len0 - len1;
+ }
+
+ cmplen = len0; /* This avoids comparing ending quote */
+ } else {
+ cmplen = n;
+ }
+
+ for (i = 0; i < cmplen ; i++, str0++, str1++) {
+ if (my_tolower(*str0) != my_tolower(*str1)) {
return ((int) (unsigned char) my_tolower(*str0))
- ((int) (unsigned char) my_tolower(*str1));
}
- c = *str0;
- str0 += (*str1 != '"');
- str1 += (c != '"');
}
+
+ /* All characters compared and all matched */
return 0;
}
diff -Nurd -X.diff_ignore freeciv/utility/support.h freeciv/utility/support.h
--- freeciv/utility/support.h 2008-01-15 04:53:30.000000000 +0200
+++ freeciv/utility/support.h 2008-04-08 17:22:18.000000000 +0300
@@ -74,6 +74,8 @@
int mystrncasecmp(const char *str0, const char *str1, size_t n);
int mystrncasequotecmp(const char *str0, const char *str1, size_t n);
+size_t effectivestrlenquote(const char *str);
+
char *mystrcasestr(const char *haystack, const char *needle);
const char *mystrerror(void);
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
