search

ome / omero-iviewer

An OMERO.web app allowing to view images
https://www.openmicroscopy.org/omero/iviewer/
Other
19 stars 30 forks source link

Handling of invalid polygon points data (and invalid ROI/shape data in general) #421

Open knabar opened 2 years ago

knabar commented 2 years ago

We just had a situation where polygon shape data was generated by a third-party script and the points string was incorrectly formatted (with extra commas) x1,y1, x2,y2, x3,y3, ... instead of x1,y1 x2,y2 x3,y3 ... as described in the XSD specifications for Polygon (https://www.openmicroscopy.org/Schemas/OME/2016-06/ome.xsd):

The points of the Polygon are defined as a list of comma separated x,y coordinates seperated [sic] by spaces like “x1,y1 x2,y2 x3,y3” e.g. “0,0 1,2 3,5"

While this broke one of our tools, iviewer still showed the shape correctly.

Our fix probably will be not to relax the data parsing, but to catch and report invalid data to users, so they are aware and can correct it.

Is iviewer loading the shape anyway purposeful or coincidental, i.e. would the iviewer behavior in this case be considered a bug?

will-moore commented 2 years ago

I didn't write that originally, but just trying to search the code for where this is handled, I find https://github.com/ome/omero-iviewer/blob/4f4b1ce27b62c8eb95717f27b888ed59e503fbd4/src/viewers/viewer/utils/Conversion.js#L911 which seems to only handle Polygon points without the extra commas? I'll need to script some Polygon creation to test further. Certainly other places in web, e.g. the old viewer was able to handle extra commas format, since I think that's what Insight used to produce. I guess it could be considered a bug, if it gave someone the impression that this format was "valid".

knabar commented 2 years ago

@will-moore I think iviewer works right now because extra commas just produce extra elements in the c array, which are ignored by the subsequent code. It makes sure than c has at least two elements, but more than two elements are fine.