retreat: Reject invalid retreat paths

Undocumented misfeature: retreat and lretreat accept anything as
retreat path.  The paths' actual consumers retreat_ship1() and
retread_land1() silently ignore invalid direction characters.

The retreat paths are in xdump, and invalid ones could conceivably
confuse smart clients.

Change the commands to reject invalid paths, and the consumers to oops
on invalid direction characters.

Note that invalid paths get rejected even when they're not actually
used because the conditions argument contains a "c" for "cancel".
Requiring the user give a new path so he can cancel the old one is
comically bad design.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>

diff --git a/src/lib/commands/retr.c b/src/lib/commands/retr.c
index 9bf6f2396c087547c440400f90eb238e0bc6fad4..562af9777758a2f6bdc4a63b9e24c4c0ab2db3d3 100644 (file)
--- a/src/lib/commands/retr.c
+++ b/src/lib/commands/retr.c
@@ -38,6 +38,7 @@
 #include "commands.h"
 #include "empobj.h"
 #include "land.h"
+#include "path.h"
 #include "retreat.h"
 #include "ship.h"
 
@@ -84,9 +85,16 @@ retreat(int type)
     if (!snxtitem(&ni, type, player->argp[1], NULL))
        return RET_SYN;
     nunits = 0;
-    if (player->argp[2] != NULL)
+    if (player->argp[2] != NULL) {
        pq = getstarg(player->argp[2], "Retreat path? ", buf1);
-    else
+       for (i = 0; i < RET_LEN - 1 && pq[i]; i++) {
+           if (chkdir(pq[i], DIR_STOP, DIR_LAST) < 0) {
+               pr("'%c' is not a valid direction...\n", pq[i]);
+               direrr(NULL, NULL, NULL);
+               return RET_SYN;
+           }
+       }
+    } else
        pq = NULL;
 
     rflags = 0;

diff --git a/src/lib/subs/retreat.c b/src/lib/subs/retreat.c
index 97e982aa04ce1b56aa8ffeb25addd9dc419c250b..d850aa0c1b7eede4fc7e9ec87d1c180f6c031075 100644 (file)
--- a/src/lib/subs/retreat.c
+++ b/src/lib/subs/retreat.c
@@ -202,9 +202,7 @@ retreat_ship1(struct shpstr *sp, char code, int orig)
        }
        dir = chkdir(sp->shp_rpath[0], DIR_STOP, DIR_LAST);
        memmove(sp->shp_rpath, sp->shp_rpath+1, sizeof(sp->shp_rpath) - 1);
-       if (dir < 0)
-           continue;
-       if (dir == DIR_STOP)
+       if (dir == DIR_STOP || CANT_HAPPEN(dir < 0))
            break;
        dx = diroff[dir][0];
        dy = diroff[dir][1];
@@ -386,9 +384,7 @@ retreat_land1(struct lndstr *lp, char code, int orig)
        }
        dir = chkdir(lp->lnd_rpath[0], DIR_STOP, DIR_LAST);
        memmove(lp->lnd_rpath, lp->lnd_rpath+1, sizeof(lp->lnd_rpath) - 1);
-       if (dir < 0)
-           continue;
-       if (dir == DIR_STOP)
+       if (dir == DIR_STOP || CANT_HAPPEN(dir < 0))
            break;
        dx = diroff[dir][0];
        dy = diroff[dir][1];

diff --git a/tests/retreat/01-retreat-1 b/tests/retreat/01-retreat-1
index ad55e03aa89d4d0fd591c5517f0157ab5e106b18..8dae7de892f5de4a4ffe3e59d169e9a4bc496ed2 100644 (file)
--- a/tests/retreat/01-retreat-1
+++ b/tests/retreat/01-retreat-1
@@ -2,6 +2,7 @@
 || retreat command
 | garbage path
 retr 0 garbage i
+__cmd added 0 -1 0
 | condition help, then junk conditions
 retr 1 g
 ?
@@ -9,7 +10,7 @@ junk
 | long path, all conditions
 retr 2/3 bgyujnbgyujnbgyujn itshbdu
 | clear retreat order
-retr 3 ignored c
+retr 3 h c
 | group retreat order
 retr a ?uid=4 b h
 | show retreat orders
@@ -18,6 +19,7 @@ retr
 || lretreat command
 | garbage path
 lret 0 garbage i
+__cmd added 0 -1 0
 | condition help, then junk conditions
 lret 1 g
 ?
@@ -25,7 +27,7 @@ junk
 | long path, all conditions
 lret 2/3 yujnbgyujnbgyujnbg ihb
 | clear retreat order
-lret 3 ignored c
+lret 3 h c
 | group retreat order
 lret a ?uid=4 b h
 | show retreat orders

diff --git a/tests/retreat/final.xdump b/tests/retreat/final.xdump
index bef3571676f59ccd2b13d400a291175b2c8b5959..e8fa5cf9f631352fb52a5dabbfe7ced594edd991 100644 (file)
--- a/tests/retreat/final.xdump
+++ b/tests/retreat/final.xdump
@@ -147,7 +147,7 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd
 /config
 config ship
 uid owner xloc yloc type effic mobil off tech opx opy mission radius fleet xstart xend ystart yend cargostart(0) cargostart(1) cargostart(2) cargostart(3) cargostart(4) cargostart(5) cargoend(0) cargoend(1) cargoend(2) cargoend(3) cargoend(4) cargoend(5) amtstart(0) amtstart(1) amtstart(2) amtstart(3) amtstart(4) amtstart(5) amtend(0) amtend(1) amtend(2) amtend(3) amtend(4) amtend(5) autonav civil milit shell gun petrol iron dust bar food oil lcm hcm uw rad pstage ptime access mquota path follow name xbuilt ybuilt builder rflags rpath
-0 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 0 "" -3 1 1 (injured) "garbage"
+0 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 0 "" -3 1 1 () ""
 1 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 1 "" -3 1 1 () ""
 2 1 -3 1 2 91 115 0 20 0 0 none 0 "" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 9 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 2 "" -3 1 1 (injured torped sonared helpless bombed depth-charged boarded) "bgyujnbgy"
 3 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 3 "" -3 1 1 () ""
@@ -353,7 +353,7 @@ uid owner xloc yloc type effic mobil off tech opx opy mission radius wing range
 /config
 config land
 uid owner xloc yloc type effic mobil off tech opx opy mission radius army ship harden retreat rflags rpath civil milit shell gun petrol iron dust bar food oil lcm hcm uw rad pstage ptime land access
-0 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 (injured) "garbage" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0
+0 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0
 1 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0
 2 1 -3 1 2 89 113 0 50 0 0 none 0 "" -1 127 42 (injured helpless bombed) "yujnbgyuj" 0 9 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0
 3 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0

diff --git a/tests/retreat/journal.log b/tests/retreat/journal.log
index 93a46154426335257d46ddbb6026a72da62a9fb4..ab6364031c555553cea0ab7b06f01f4f92f7f9b7 100644 (file)
--- a/tests/retreat/journal.log
+++ b/tests/retreat/journal.log
@@ -22,9 +22,15 @@
     Play#1 output Play#1 6 0 640
     Play#1 input retr 0 garbage i
     Play#1 command retreat
-    Play#1 output Play#1 1 shp#     ship type       x,y   fl path       as flt?  flags
-    Play#1 output Play#1 1    0 cs   cargo ship    -3,1    a garbage             i
-    Play#1 output Play#1 1 1 ship
+    Play#1 output Play#1 1 'a' is not a valid direction...
+    Play#1 output Play#1 1 Legal directions are:
+    Play#1 output Play#1 1  y u
+    Play#1 output Play#1 1 g   j
+    Play#1 output Play#1 1  b n
+    Play#1 output Play#1 1 Usage: retreat <SHIPS|FLEET> <PATH> [i|t|s|h|b|d|u|c]
+    Play#1 output Play#1 6 0 640
+    Play#1 input __cmd added 0 -1 0
+    Play#1 command __cmd
     Play#1 output Play#1 6 0 639
     Play#1 input retr 1 g
     Play#1 command retreat
@@ -50,7 +56,7 @@
     Play#1 output Play#1 1    3 cs   cargo ship    -3,1    a bgyujnbgy           itshbdu
     Play#1 output Play#1 1 2 ships
     Play#1 output Play#1 6 0 638
-    Play#1 input retr 3 ignored c
+    Play#1 input retr 3 h c
     Play#1 command retreat
     Play#1 output Play#1 1 shp#     ship type       x,y   fl path       as flt?  flags
     Play#1 output Play#1 1    3 cs   cargo ship    -3,1    a
@@ -67,7 +73,7 @@
     Play#1 output Play#1 4 ship(s)?
     Play#1 input 0/1/2/3/4
     Play#1 output Play#1 1 shp#     ship type       x,y   fl path       as flt?  flags
-    Play#1 output Play#1 1    0 cs   cargo ship    -3,1    a garbage             i
+    Play#1 output Play#1 1    0 cs   cargo ship    -3,1    a
     Play#1 output Play#1 1    1 cs   cargo ship    -3,1    a
     Play#1 output Play#1 1    2 cs   cargo ship    -3,1      bgyujnbgy           itshbdu
     Play#1 output Play#1 1    3 cs   cargo ship    -3,1    a
@@ -76,9 +82,15 @@
     Play#1 output Play#1 6 0 635
     Play#1 input lret 0 garbage i
     Play#1 command lretreat
-    Play#1 output Play#1 1 lnd#     unit type       x,y   ar path       as army? flags
-    Play#1 output Play#1 1    0 inf  infantry      -3,1    a garbage             i
-    Play#1 output Play#1 1 1 unit
+    Play#1 output Play#1 1 'a' is not a valid direction...
+    Play#1 output Play#1 1 Legal directions are:
+    Play#1 output Play#1 1  y u
+    Play#1 output Play#1 1 g   j
+    Play#1 output Play#1 1  b n
+    Play#1 output Play#1 1 Usage: lretreat <UNITS|ARMY> <PATH> [i|h|b|c]
+    Play#1 output Play#1 6 0 635
+    Play#1 input __cmd added 0 -1 0
+    Play#1 command __cmd
     Play#1 output Play#1 6 0 634
     Play#1 input lret 1 g
     Play#1 command lretreat
@@ -100,7 +112,7 @@
     Play#1 output Play#1 1    3 inf  infantry      -3,1    a yujnbgyuj           ihb
     Play#1 output Play#1 1 2 units
     Play#1 output Play#1 6 0 633
-    Play#1 input lret 3 ignored c
+    Play#1 input lret 3 h c
     Play#1 command lretreat
     Play#1 output Play#1 1 lnd#     unit type       x,y   ar path       as army? flags
     Play#1 output Play#1 1    3 inf  infantry      -3,1    a
@@ -117,7 +129,7 @@
     Play#1 output Play#1 4 land(s)?
     Play#1 input 0/1/2/3/4
     Play#1 output Play#1 1 lnd#     unit type       x,y   ar path       as army? flags
-    Play#1 output Play#1 1    0 inf  infantry      -3,1    a garbage             i
+    Play#1 output Play#1 1    0 inf  infantry      -3,1    a
     Play#1 output Play#1 1    1 inf  infantry      -3,1    a
     Play#1 output Play#1 1    2 inf  infantry      -3,1      yujnbgyuj           ihb
     Play#1 output Play#1 1    3 inf  infantry      -3,1    a