Complications on the watch, and a Wear artifact beside the phone APK

CodenameOne/src/com/codename1/surfaces/SurfaceRasterizer.java

@@ -251,8 +251,13 @@ public static long nextEntryFlip(Map<String, Object> timelineDoc, long now) {
     }
 
     /// Picks the layout of a timeline document for a size name (`small` / `medium` / `large` /
-    /// `lockscreen`): the explicit per-size layout when present, else the `default` layout, else
-    /// null.
+    /// `lockscreen` / the `watch*` complication families): the explicit per-size layout when
+    /// present, else a family-specific substitute, else the `default` layout, else null.
+    ///
+    /// Two substitutions, matching what the platform renderers do so a preview and a device
+    /// agree. `watchCorner` falls back to `watchCircular`, because a corner complication is
+    /// round and Wear OS has no corner slot at all; `watchRectangular` falls back to
+    /// `lockscreen`, which is the same WidgetKit family on Apple.
     ///
     /// #### Parameters
     ///
@@ -274,6 +279,17 @@ public static Map<String, Object> layoutForSize(Map<String, Object> timelineDoc,
         }
         Map<String, Object> layouts = (Map<String, Object>) layoutsObj;
         Object layout = sizeName == null ? null : layouts.get(sizeName);
+        if (!(layout instanceof Map) && sizeName != null) {
+            String substitute = null;
+            if ("watchCorner".equals(sizeName)) {
+                substitute = "watchCircular";
+            } else if ("watchRectangular".equals(sizeName)) {
+                substitute = "lockscreen";
+            }
+            if (substitute != null) {
+                layout = layouts.get(substitute);
+            }
+        }
         if (!(layout instanceof Map)) {
             layout = layouts.get("default");
         }
@@ -282,15 +298,23 @@ public static Map<String, Object> layoutForSize(Map<String, Object> timelineDoc,
 
     // --- dynamic text ----------------------------------------------------------
 
-    /// Formats a dynamic-text value the way the OS-native views would show it. Package-private so
-    /// unit tests can cover the formatting without a `Display`.
+    /// Formats a dynamic-text value the way the OS-native views would show it.
+    ///
+    /// Public because a surface that cannot tick natively needs the text form: a Wear
+    /// complication slot takes a string, and a Tile freezes its value between timeline flips.
+    /// Both go through this rather than formatting for themselves, so a countdown reads the same
+    /// on a watch face as in the simulator preview and on a home screen.
     ///
     /// #### Parameters
     ///
     /// - `style`: the wire style name (`timerDown`, `timerUp`, `time`, `date`, `relative`)
     /// - `dateMillis`: the target epoch millis
     /// - `now`: the current epoch millis
-    static String formatDynamicText(String style, long dateMillis, long now) {
+    ///
+    /// #### Returns
+    ///
+    /// the formatted value
+    public static String formatDynamicText(String style, long dateMillis, long now) {
         if ("timerUp".equals(style)) {
             return formatTimer(now - dateMillis);
         }

CodenameOne/src/com/codename1/surfaces/SurfaceSerializer.java

@@ -359,7 +359,10 @@ private static byte[] encode(Image img) {
 
     private static final char[] HEX_DIGITS = "0123456789abcdef".toCharArray();
 
-    private static String fnv1a(byte[] data) {
+    /// Package-visible so `Surfaces.publishRemote` can check that a name a server supplied
+    /// really is the hash of the bytes beside it. One implementation, because two would
+    /// eventually disagree and the disagreement would look like corruption.
+    static String fnv1a(byte[] data) {
         long hash = 0xcbf29ce484222325L;
         for (byte b : data) {
             hash ^= b & 0xff;

CodenameOne/src/com/codename1/surfaces/Surfaces.java

@@ -28,6 +28,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -197,19 +198,171 @@ public static void publish(String kindId, WidgetTimeline timeline) {
         }
         Map<String, byte[]> images = new LinkedHashMap<String, byte[]>();
         String json = SurfaceSerializer.serializeTimeline(kindId, timeline, images);
-        b.publishWidgetTimeline(kindId, json, images);
+        synchronized (publishLock(kindId)) {
+            b.publishWidgetTimeline(kindId, json, images);
+        }
+    }
+
+    /// One monitor per kind, created on demand and never removed. Kind ids come from
+    /// surfaces.json, so the set is bounded by the app's own declaration.
+    private static final Map<String, Object> PUBLISH_LOCKS = new HashMap<String, Object>();
+
+    /// The monitor that serializes publishes of a single kind.
+    ///
+    /// A publish is a WRITE FOLLOWED BY A HAND-OFF, and the two are only meaningful as a pair:
+    /// the platform replaces the timeline in its container and then gives the same descriptor to
+    /// the watch. Let two publishes of one kind interleave and the later write can be paired with
+    /// the earlier hand-off, so the watch is left holding a descriptor the phone has already
+    /// replaced -- and left holding it for good, because nothing publishes again to correct it.
+    /// The imagery is worse than stale rather than merely old: both platforms read the blobs back
+    /// off disk at hand-off time, so the descriptor of one publish can be sent with the artwork of
+    /// another, which is a pairing neither publish ever produced.
+    ///
+    /// publish() documents itself as callable from any thread, so two threads publishing one kind
+    /// is a supported way to call this rather than an abuse of it.
+    ///
+    /// Per KIND rather than one global monitor: a publish is file I/O plus a synchronous native
+    /// call, and two different kinds have nothing to say to each other.
+    private static Object publishLock(String kindId) {
+        synchronized (PUBLISH_LOCKS) {
+            Object lock = PUBLISH_LOCKS.get(kindId);
+            if (lock == null) {
+                lock = new Object();
+                PUBLISH_LOCKS.put(kindId, lock);
+            }
+            return lock;
+        }
     }
 
     /// Push-framework entry point for a server-rendered timeline descriptor. The descriptor uses
     /// the same wire format as `publish()`. The descriptor is persisted directly once the
     /// Codename One runtime receives it. A platform that doesn't run application code for a
     /// background push applies it when the application next starts or resumes.
+    ///
+    /// Equivalent to [#publishRemote(String,String,Map)] with no imagery. A descriptor that
+    /// references an image by name renders a gap where it should be, so prefer the overload
+    /// whenever the artwork travelled with the descriptor.
     public static void publishRemote(String kindId, String timelineJson) {
+        publishRemote(kindId, timelineJson, Collections.<String, byte[]>emptyMap());
+    }
+
+    /// As [#publishRemote(String,String)], with the imagery the descriptor references.
+    ///
+    /// A timeline's node tree names its images rather than embedding them -- `SurfaceSerializer`
+    /// hashes the bytes and puts the hash on the wire -- so a descriptor that arrived from
+    /// somewhere else is only complete if its side-map arrived too. Without this overload
+    /// `publishRemote` discarded the imagery unconditionally and every referenced image rendered
+    /// as a gap.
+    ///
+    /// The two callers are a server push and the phone-to-watch mirror, which forwards a
+    /// phone-side `publish()` of a watch-bearing kind to the watch. Both are the same operation:
+    /// a descriptor produced elsewhere, applied here.
+    ///
+    /// #### Parameters
+    ///
+    /// - `kindId`: the widget kind id
+    /// - `timelineJson`: the serialized timeline, in the same wire format `publish()` produces
+    /// - `images`: the referenced images by name, or an empty map when the descriptor names none
+    public static void publishRemote(String kindId, String timelineJson,
+            Map<String, byte[]> images) {
         SurfaceBridge b = bridgeInternal();
         if (b == null || !b.areWidgetsSupported() || kindId == null || timelineJson == null) {
             return;
         }
-        b.publishWidgetTimeline(kindId, timelineJson, Collections.<String, byte[]>emptyMap());
+        // The KIND is input here too, and a worse one to get wrong than an image name: every
+        // platform composes it into a directory path -- iOS as container + "/cn1surfaces/" +
+        // kindId -- so "../activities/foo" writes the timeline AND its imagery outside the kind
+        // directory, over whatever is there. publish() cannot produce such an id because
+        // WidgetKind refuses it at construction; a descriptor that arrived from a server or from
+        // the watch mirror never passed through that check, so it gets it here. The same
+        // validator, not a second copy of the grammar.
+        if (!WidgetKind.isValidId(kindId)) {
+            Log.p("Surfaces: refusing a remote publish for a kind id that is not [a-z][a-z0-9_]*: "
+                    + kindId);
+            return;
+        }
+        // The same monitor publish() uses: a remote descriptor and a local one race exactly the
+        // same way, and a push landing while the app publishes is the ordinary way it happens.
+        synchronized (publishLock(kindId)) {
+            b.publishWidgetTimeline(kindId, timelineJson, safeImageNames(images));
+        }
+    }
+
+    /// The image side-map with anything that is not a plain blob name removed.
+    ///
+    /// A name here is a content hash produced by `SurfaceSerializer`, and every platform turns it
+    /// into a file inside the kind's own directory. This descriptor did NOT come from this
+    /// process, though -- a server push and the watch mirror both arrive from outside -- so the
+    /// names are input, not something the app computed. A name carrying a separator or a parent
+    /// segment escapes that directory: on iOS the path is composed as `dir + "/" + key + ".png"`
+    /// with no sanitizing of its own, so `../other_kind/hash` plants a blob under a different
+    /// kind, where a later legitimate publish will not replace it -- content-hash names are
+    /// assumed to already hold the right bytes.
+    ///
+    /// Dropped rather than rejected wholesale: a descriptor referencing an image that did not
+    /// arrive renders a gap, which every renderer already tolerates, and refusing the whole
+    /// publish would let one bad name suppress a timeline that is otherwise fine.
+    /// The prefix `SurfaceSerializer.registerImageBytes` puts in front of a content hash.
+    private static final String CONTENT_HASH_PREFIX = "img";
+
+    /// Whether a name has the shape SurfaceSerializer gives a content hash: the `img` prefix and
+    /// sixteen lowercase hex digits. The prefix is the point -- checking for bare hex matched
+    /// nothing the framework produces, so the integrity check below never ran on a real payload
+    /// at all, and would have compared a prefixed name against an unprefixed hash if it had.
+    ///
+    /// Only names of this shape are verified, so one that was never a hash -- a registered image
+    /// the app named itself -- is passed through rather than refused for failing a test that does
+    /// not apply to it.
+    private static boolean looksLikeContentHash(String name) {
+        if (name.length() != CONTENT_HASH_PREFIX.length() + 16
+                || !name.startsWith(CONTENT_HASH_PREFIX)) {
+            return false;
+        }
+        for (int i = CONTENT_HASH_PREFIX.length(); i < name.length(); i++) {
+            char c = name.charAt(i);
+            if ((c < '0' || c > '9') && (c < 'a' || c > 'f')) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private static Map<String, byte[]> safeImageNames(Map<String, byte[]> images) {
+        if (images == null || images.isEmpty()) {
+            return Collections.<String, byte[]>emptyMap();
+        }
+        Map<String, byte[]> safe = new LinkedHashMap<String, byte[]>();
+        for (Map.Entry<String, byte[]> e : images.entrySet()) {
+            String name = e.getKey();
+            if (name == null || name.length() == 0 || name.indexOf('/') >= 0
+                    || name.indexOf('\\') >= 0 || name.indexOf(':') >= 0
+                    || name.indexOf('\0') >= 0 || ".".equals(name) || "..".equals(name)) {
+                Log.p("Surfaces: dropping a remote image whose name is not a plain blob name: "
+                        + name);
+                continue;
+            }
+            if (looksLikeContentHash(name) && e.getValue() != null
+                    && !name.equals(CONTENT_HASH_PREFIX + SurfaceSerializer.fnv1a(e.getValue()))) {
+                // The name is a CLAIM about the bytes, and this descriptor came from outside the
+                // process. iOS skips writing a blob whose file already exists, on the strength of
+                // that claim -- so bad bytes landing first cannot be repaired by any later
+                // legitimate publish, and the surface shows wrong artwork for good. Checking the
+                // claim costs one pass over bytes that are about to be written anyway.
+                Log.p("Surfaces: dropping a remote image whose bytes do not match its name: "
+                        + name);
+                continue;
+            }
+            if (e.getValue() == null) {
+                // A name with no bytes -- one attachment of several failing to decode is the
+                // ordinary way to get one. Android skips a null value; the iOS bridge writes it
+                // straight to an OutputStream and the NullPointerException escapes its IOException
+                // catch, so one missing blob aborted a publish whose timeline was otherwise fine.
+                Log.p("Surfaces: dropping a remote image with no bytes: " + name);
+                continue;
+            }
+            safe.put(name, e.getValue());
+        }
+        return safe;
     }
 
     /// Asks the platform to re-render widgets from their already-published timelines.

CodenameOne/src/com/codename1/surfaces/WidgetKind.java

@@ -51,7 +51,11 @@ public WidgetKind(String id) {
         this.id = id;
     }
 
-    private static boolean isValidId(String id) {
+    /// Whether an id matches the documented `[a-z][a-z0-9_]*` grammar.
+    ///
+    /// Package-visible because `Surfaces.publishRemote` has to apply the same rule to an id that
+    /// arrived from outside the process, and two copies of a grammar is how they come to disagree.
+    static boolean isValidId(String id) {
         int n = id.length();
         if (n == 0) {
             return false;