Merge pull request #284 from ennorehling/feature/bug-1685-give-invalid-target

Bug 1685: syntax errors should not give items to peasants

src/give.test.c

@@ -296,6 +296,31 @@ static void test_give_denied_by_rules(CuTest * tc) {
     test_cleanup();
 }
 
+static void test_give_invalid_target(CuTest *tc) {
+    // bug https://bugs.eressea.de/view.php?id=1685
+    struct give env;
+    order *ord;
+    struct locale * lang;
+
+    test_cleanup();
+    env.f1 = test_create_faction(0);
+    env.f2 = 0;
+    setup_give(&env);
+
+    i_change(&env.src->items, env.itype, 10);
+    lang = get_or_create_locale("test");
+    env.f1->locale = lang;
+    locale_setstring(lang, "KRAEUTER", "HERBS");
+    init_locale(lang);
+    ord = create_order(K_GIVE, lang, "## HERBS");
+    assert(ord);
+
+    give_cmd(env.src, ord);
+    CuAssertIntEquals(tc, 10, i_get(env.src->items, env.itype));
+    CuAssertPtrNotNull(tc, test_find_messagetype(env.f1->msgs, "feedback_unit_not_found"));
+    test_cleanup();
+}
+
 CuSuite *get_give_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
@@ -315,5 +340,6 @@ CuSuite *get_give_suite(void)
     SUITE_ADD_TEST(suite, test_give_herbs);
     SUITE_ADD_TEST(suite, test_give_okay);
     SUITE_ADD_TEST(suite, test_give_denied_by_rules);
+    SUITE_ADD_TEST(suite, test_give_invalid_target);
     return suite;
 }

src/kernel/config.c

@@ -768,7 +768,7 @@ int read_unitid(const faction * f, const region * r)
      * paramliste. machen wir das nicht, dann wird getnewunit in s nach der
      * nummer suchen, doch dort steht bei temp-units nur "temp" drinnen! */
 
-    if (!s || *s == 0) {
+    if (!s || *s == 0 || !isalnum(*s)) {
         return -1;
     }
     if (isparam(s, f->locale, P_TEMP)) {
@@ -842,6 +842,7 @@ building *largestbuilding(const region * r, cmp_building_cb cmp_gt,
 extern faction *dfindhash(int i);
 
 static const char *forbidden[] = { "t", "te", "tem", "temp", NULL };
+// PEASANT: "b", "ba", "bau", "baue", "p", "pe", "pea", "peas"
 
 int forbiddenid(int id)
 {
@@ -1025,6 +1026,16 @@ typedef struct param {
     char *data;
 } param;
 
+void free_params(struct param **pp) {
+    while (*pp) {
+        param *p = *pp;
+        free(p->name);
+        free(p->data);
+        *pp = p->next;
+        free(p);
+    }
+}
+
 const char *get_param(const struct param *p, const char *key)
 {
     while (p != NULL) {

src/kernel/config.h

@@ -277,6 +277,7 @@ extern "C" {
     int get_param_int(const struct param *p, const char *key, int def);
     int check_param(const struct param *p, const char *key, const char *searchvalue);
     float get_param_flt(const struct param *p, const char *key, float def);
+    void free_params(struct param **pp);
 
     bool ExpensiveMigrants(void);
     int NMRTimeout(void);

src/kernel/config.test.c

@@ -14,6 +14,51 @@
 
 struct critbit_tree;
 
+static void test_read_unitid(CuTest *tc) {
+    unit *u;
+    order *ord;
+    attrib *a;
+    struct locale *lang;
+    struct terrain_type *t_plain;
+
+    test_cleanup();
+    lang = get_or_create_locale("de");
+    test_translate_param(lang, P_TEMP, "TEMP");
+    /* note that the english order is FIGHT, not COMBAT, so this is a poor example */
+    t_plain = test_create_terrain("plain", LAND_REGION);
+    u = test_create_unit(test_create_faction(0), test_create_region(0, 0, t_plain));
+    a = a_add(&u->attribs, a_new(&at_alias));
+    a->data.i = atoi36("42"); /* this unit is also TEMP 42 */
+
+    ord = create_order(K_GIVE, lang, "TEMP 42");
+    init_order(ord);
+    CuAssertIntEquals(tc, u->no, read_unitid(u->faction, u->region));
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, "8");
+    init_order(ord);
+    CuAssertIntEquals(tc, 8, read_unitid(u->faction, u->region));
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, "");
+    init_order(ord);
+    CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region));
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, "TEMP");
+    init_order(ord);
+    CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region));
+    free_order(ord);
+
+    // bug https://bugs.eressea.de/view.php?id=1685
+    ord = create_order(K_GIVE, lang, "##");
+    init_order(ord);
+    CuAssertIntEquals(tc, -1, read_unitid(u->faction, u->region));
+    free_order(ord);
+
+    test_cleanup();
+}
+
 static void test_getunit(CuTest *tc) {
     unit *u, *u2;
     order *ord;
@@ -53,6 +98,20 @@ static void test_getunit(CuTest *tc) {
     CuAssertPtrEquals(tc, NULL, u2);
     free_order(ord);
 
+    // bug https://bugs.eressea.de/view.php?id=1685
+    ord = create_order(K_GIVE, lang, "TEMP ##");
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_NOTFOUND, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, NULL, u2);
+    free_order(ord);
+
+    // bug https://bugs.eressea.de/view.php?id=1685
+    ord = create_order(K_GIVE, lang, "##");
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_NOTFOUND, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, NULL, u2);
+    free_order(ord);
+
     ord = create_order(K_GIVE, lang, "TEMP 42");
     init_order(ord);
     CuAssertIntEquals(tc, GET_UNIT, getunit(u->region, u->faction, &u2));
@@ -97,10 +156,22 @@ static void test_param_flt(CuTest * tc)
     CuAssertDblEquals(tc, 42.0, get_param_flt(par, "bar", 0.0), 0.01);
 }
 
+static void test_forbiddenid(CuTest *tc) {
+    CuAssertIntEquals(tc, 0, forbiddenid(1));
+    CuAssertIntEquals(tc, 1, forbiddenid(0));
+    CuAssertIntEquals(tc, 1, forbiddenid(-1));
+    CuAssertIntEquals(tc, 1, forbiddenid(atoi36("temp")));
+    CuAssertIntEquals(tc, 1, forbiddenid(atoi36("tem")));
+    CuAssertIntEquals(tc, 1, forbiddenid(atoi36("te")));
+    CuAssertIntEquals(tc, 1, forbiddenid(atoi36("t")));
+}
+
 CuSuite *get_config_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_forbiddenid);
     SUITE_ADD_TEST(suite, test_getunit);
+    SUITE_ADD_TEST(suite, test_read_unitid);
     SUITE_ADD_TEST(suite, test_get_set_param);
     SUITE_ADD_TEST(suite, test_param_int);
     SUITE_ADD_TEST(suite, test_param_flt);

src/report.c

@@ -2448,21 +2448,6 @@ const char *charset)
     return 0;
 }
 
-void base36conversion(void)
-{
-    region *r;
-    for (r = regions; r; r = r->next) {
-        unit *u;
-        for (u = r->units; u; u = u->next) {
-            if (forbiddenid(u->no)) {
-                uunhash(u);
-                u->no = newunitid();
-                uhash(u);
-            }
-        }
-    }
-}
-
 #define FMAXHASH 1021
 
 struct fsee {

src/tests.c

@@ -73,6 +73,7 @@ void test_cleanup(void)
     free_resources();
     global.functions.maintenance = NULL;
     global.functions.wage = NULL;
+    free_params(&global.parameters);
     default_locale = 0;
     free_locales();
     free_spells();

src/util/base36.c

@@ -32,9 +32,6 @@ int atoi36(const char *str)
     assert(s);
     if (!(*s))
         return 0;
-
-    while (isxspace(*(unsigned char *)s))
-        ++s;
     if (*s == '-') {
         sign = -1;
         ++s;