Closed meletis closed 6 years ago
It's hard to know whether the right sequence is XForms->XLSForm (thus beginning here with an issue in this repo) -- or if there's somewhere even higher-level to discuss the high-level UX needs first, before coming to the specs with more concrete proposals for deviations. But let's see, this is an experiment in how we can collaborate with the broader community on features that are likely valuable far beyond the SurveyCTO user base.
@MartijnR, would love your input. And hmm, I can't seem to tag mberg because I guess maybe he's not a party to this repo. @lognaturel, if we want to try to pull mberg into this discussion, does that mean that we should post something to https://github.com/XLSForm/xlsform.github.io? (Apologies for the newbie questions.)
@meletis @chrislrobert Sounds like a good idea that would address a few issues filed on the Collect side as well (e.g. opendatakit/collect#236). I like the idea of giving the form designer control on a per-image basis.
I don't really understand why some GitHub usernames autocomplete and others don't. You may be right that it's based on who is an organization member. Regardless, you can absolutely tag @mberg and I do hope he'll join this party. 😊🎉
Enketo has also come across this requirement occasionally (and not addressed it). I haven't found an existing approach that actually transforms the image (Orbeon has a nice way of adding a constraint for file sizes though). Maybe Dimagi has one, @ctsims, or Esri, @spatialdude?
If there is no existing XForms approach, the only general approaches (that do not abuse appearances ;)), I've come up with so far are:
a. A custom form-wide attribute on the <submission>
element.
b. A custom bind attribute.
We'd miss out on the fine-grained per-question approach with option A, but I wonder if that is really necessary.
Are just talking about images? Videos and audio, are quite a different case I think, because you'd probably want to constrain (not just transform) the length of the recording as well to have some control of the final quality of the recording. But we could keep that future audio/video requirement in mind when designing the syntax.
I think it makes sense to focus on images for this but agree that it would be good to have a path for eventually including analogous features for audio/video. @chrislrobert your users' immediate needs are focused on images, right?
I also looked at Orbeon forms and the vast W3C XForms spec looking for examples of data transformations and am not finding very much. One example is Orbeon's custom xxf:whitespace
model item property which I think is the same as what @MartijnR refers to as a bind attribute in his option b.
Thanks, @MartijnR, and great find on the whitespace bind, @lognaturel. I think that the custom-bind approach (a la xxf:whitespace) would be a bit more flexible in that it would allow flexibility to save different images with different resolutions. For example, if I'm taking a picture of the clinic's roof to get an idea of building materials, I can let that be really small; but if I'm taking a photo of a birth register page, I might want that pretty large so that the text can be read for review. Also, if we add it field-by-field, then it could be easier to add support for videos and audio later, without people who use the form-wide option having to fear that one day the option will start magically transforming other fields. Finally, form designers and XLSForm would still have the flexibility to auto-add to all image fields or let the user choose field-by-field. Ultimately, the bind approach seems quite flexible to me.
We'll then have to debate again how to add new binds to XLSForm in a user-friendly way, hopefully without a new column for every possible bind. But I actually think there's been headway on that discussion and maybe a not-terrible approach that's been used by Nafundi and Ona? (I could be making that up.) We can settle on the XForms part here, though, and then create a new XLSForm issue to discuss how to design that part?
If we did add a bind attribute, what would we name it? In both Android and JS, it seems easiest to scale to pixels, and we could do something like "maxpixels". We initially thought "maxwidth" would be nice, but the largest dimension depends on the orientation and so that seemed ambiguous.
Oh nice, I hadn't actually found xxf:whitespace
at all! :smiley: (I was referring to xxf:size
but didn't link to it because it's only used for image size validation, not size transformation so it's not really applicable). Very good find indeed!
Per-question control via binds is fine with me.
What sort of value could we need? Would a Mb (or byte) number value be all the client would need? Or are there different types of transformations we may want to perform (at any point in the future)? I think this may help figuring out the right name for the attribute.
[edit: sorry didn't read Chris' width suggestion above]
Agreeing on something here and then taking up the conversation on the XLSForm side sounds perfect to me. And I think we're close here!
Another nice thing about the bind approach is that the value can be the result of an XPath expression (if we decide to allow it). That would mean the size of the image can be dynamically determined by the answer to a previous question or even something like the phone type or location.
I agree with @chrislrobert that the max pixels for the long edge is probably the most useful thing to specify. Specifying a size in Mb/bytes seems really dangerous because cameras have such different output. Does the client then increase the JPG compression level? Rescale? Pixels are pretty easy for humans to reason about and give unambiguous instructions to the client. I think maxpixels
or maxPixels
sound good. I think all lowercase is more common in W3C XForms and camel case seems to be used for most JR additions. XPath seems to favor dash separation (max-pixels
). I don't like inconsistency, can you tell?
Yes, I also agree that a new bind "action" may be the reasonable thing to do.
Moreover, from our perspective, what we want to achieve here is not to constraint data, is to accept data but only after they have been transformed according to our rules. So, let's see what we have for the moment:
There is the "constraint" bind attribute. We can say that this "action" controls what data can be accepted. If it doesn't meet the constraint, it is rejected.
There is the "calculate" bind attribute. This "action" takes some data, applies some calculations and produces a result.
Wait, what? From one point of view, calculations can also be interpreted as "transformations", right?
So, here is a naive approach which we will all reject (I think): Why not create a new function like this:
resize(${image}, 400)
... and add this to a calculated field?
One of the answers that help us reject this solution is that this function would duplicate the data, we would end up with two binaries, the original one and the "calculated".
So, it starts looking obvious to me that the next thing that we need to do is to apply a "calculation" (a transformation) in place, not on some other field.
Since, neither "action" 1 nor "action" 2 above can do that for us, I would introduce a 3rd action, called "transform". Here is what I would add in the bind section:
transform=resize(., 400)
or
transform=max-pixels(., 400)
...but for some reason I like the first one more because it denotes an action.
In the future we can define more transformation functions for binary data. But this can also apply to numbers and text. For example, if you do that on a text field....:
transform=substr(${text}, 0, 10)
...that would trim the answer "in place".
Please let me what you think. It may be an over-kill for what we need (because the Javarosa engine would need to start handling byte arrays for input/output in the functions), but doesn't it look like a reasonable approach?
@meletis, thanks for the writing out such a detailed thought process! I had also given brief thought to something like a transform
attribute as a way to also address #80 but I couldn't articulate the idea well enough (a transform for that one could be barcode-as-binary
or something like that). I like what you've come up with. It's nicely analogous with constraint
, calculate
, etc.
Few quick thoughts.
I'd stick with width as that's what most image resizing libraries use. You are setting the width so it's not really a max. Also, I'm pretty sure you just set the width in libraries like imagemagick and it outputs the same size image whether is vertical or horizontal but we can double check.
The other two things we should probably add is a size and quality option.
Size works as a percentage ie) size of photo. 50% would return an image if half the pixel width of the original. It might be useful for users who don't really know what pixel width to use (we can provide suggestions) and just want to experiment.
The other one is quality. This is a percentage and determines how aggressively to compress the image. I think it most cases it defaults to 85 or 90%.
Lastly, we might want to consider some image aliases similar to what iPhoto does when you export. We should double check what they are but I think it's something like xl (1600), l, m, s (640).
Lastly, the one potential issue with resizing is it could alter or delete the exif. The exif is critical for storing the orientation in which the photo was taken. We use exifs in ona's photo gallery feature to help ensure they are displayed properly. So we might want to make sure we maintain the exif in our implementation.
Last last, it might be useful to be able to store images in black and white or grayscale images. I realize you can easily do that post production but if it saves images size when taking picture of text it might represent an optimization some projects might really like. I'm thinking mostly of projects that are using odk to take pictures of paper forms.
On Mon, Jan 30, 2017 at 7:20 AM Hélène Martin notifications@github.com wrote:
@meletis https://github.com/meletis, thanks for the writing out such a detailed thought process! I had also given brief thought to something like a transform attribute as a way to also address #80 https://github.com/opendatakit/xforms-spec/issues/80 but I couldn't articulate the idea well enough (a transform for that one could be barcode-as-binary or something like that). I like what you've come up with. It's nicely analogous with constraint, calculate, etc.
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-275977268, or mute the thread https://github.com/notifications/unsubscribe-auth/AADiQQ5chkFTDsh8rn2R056cfLuAnbMxks5rXWT8gaJpZM4Ltmza .
@meletis we should use something like resize as this is a function. It would be nice if we could pass a function in a function. Ie grayscale(resize(1200,85)) On Mon, Jan 30, 2017 at 9:11 AM Matt Berg mberg@ona.io wrote:
Few quick thoughts.
I'd stick with width as that's what most image resizing libraries use. You are setting the width so it's not really a max. Also, I'm pretty sure you just set the width in libraries like imagemagick and it outputs the same size image whether is vertical or horizontal but we can double check.
The other two things we should probably add is a size and quality option.
Size works as a percentage ie) size of photo. 50% would return an image if half the pixel width of the original. It might be useful for users who don't really know what pixel width to use (we can provide suggestions) and just want to experiment.
The other one is quality. This is a percentage and determines how aggressively to compress the image. I think it most cases it defaults to 85 or 90%.
Lastly, we might want to consider some image aliases similar to what iPhoto does when you export. We should double check what they are but I think it's something like xl (1600), l, m, s (640).
Lastly, the one potential issue with resizing is it could alter or delete the exif. The exif is critical for storing the orientation in which the photo was taken. We use exifs in ona's photo gallery feature to help ensure they are displayed properly. So we might want to make sure we maintain the exif in our implementation.
Last last, it might be useful to be able to store images in black and white or grayscale images. I realize you can easily do that post production but if it saves images size when taking picture of text it might represent an optimization some projects might really like. I'm thinking mostly of projects that are using odk to take pictures of paper forms.
On Mon, Jan 30, 2017 at 7:20 AM Hélène Martin notifications@github.com wrote:
@meletis https://github.com/meletis, thanks for the writing out such a detailed thought process! I had also given brief thought to something like a transform attribute as a way to also address #80 https://github.com/opendatakit/xforms-spec/issues/80 but I couldn't articulate the idea well enough (a transform for that one could be barcode-as-binary or something like that). I like what you've come up with. It's nicely analogous with constraint, calculate, etc.
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-275977268, or mute the thread https://github.com/notifications/unsubscribe-auth/AADiQQ5chkFTDsh8rn2R056cfLuAnbMxks5rXWT8gaJpZM4Ltmza .
Yes, if we implement this logic, then using this syntax will also be possible. The tricky part is that the existing ODK Javarosa engine (perhaps also the Enketo engine) is not using byte arrays as input/output in the functions, so that may be a huge extension to such an engine.
Moreover, a byte array alone wouldn't be enough. The engine would also like to know the encoding of the blob, so I think it is getting even trickier than I thought (for the XForms engine developers). But perhaps the encoding is something related to the engine only and not something that should matter in this spec-related only discussion.
I have the same concern as @meletis. I actually think, a normal XPath engine would only be able to access the filename (not the content) as all that a compliant XPath evaluator is aware of is the XML document. In XPath all function parameters are either converted to String, Number, Boolean or Nodeset depending on the type of function they are used in. Adding in the ability to look up a file and access an attachment (from a db or filesystem) and then being able to store the result as a binary in that db or file system at first glance seems a little crazy. There are some XPath extensions that do binary, but I believe they are all dealing with base64 strings (stored in the XML document or obtained from a read-only URI - that approach, where the result is stored as base64-encoded string would work with calculate expressions but as meletis mentioned you'd be storing both versions of the file).
For this reason, I am hoping we can do this as a static value. That doesn't mean we cannot use a complex expression, but that they are static and not XPath. Of course this has the danger of being confusing to users, unless we let pyxform create those expressions from (many) column values, and let pyxform/ODK Validate throw errors when ${element} references are used.
P.S. We should not confuse data types with transformations (that do not change the datatype). With the barcode idea, using a new data type would be the way to go, I think.
My two cents on "width" vs. "maxwidth" vs. "maxpixels" is that "width" seems pretty confusing for a few reasons. One, I don't think that anybody would ever want to stretch an image from x<y to y pixels; rather, I think that people want to constrain the maximum without causing smaller images to stretch. Two, this idea that a width might be applied to the vertical dimension if it's portrait orientation also strikes me as confusing.
On the transformation bind, making it a generic bind for transformations seems to make sense. Going a further step to make it an evaluated expression seems to add a lot of complexity. Could it be a generic bind that takes a set of well-defined attributes and/or parameters, without becoming a full-fledged expression (a la appearances)?
On Mon, Jan 30, 2017 at 11:38 AM, Martijn van de Rijdt < notifications@github.com> wrote:
I have the same concern as @meletis https://github.com/meletis. I actually think, a normal XPath engine would only be able to access the filename (not the content) as all that a compliant XPath evaluator is aware of is the XML document. In XPath all function parameters are either converted to String, Number, Boolean or Nodeset depending on the type of function they are used in. Adding in the ability to look up a file and access an attachment (from a db or filesystem) and then being able to store the result as a binary in that db or file system at first glance seems a little crazy. There are some XPath extensions that do binary, but I believe they are all dealing with base64 strings (stored in the XML document or obtained from a read-only URI).
For this reason, I am hoping we can do this as a static value. That doesn't mean we cannot use a complex expression, but that they are static and not XPath. Of course this has the danger of being confusing to users, unless we let pyxform create those expressions from (many) column values, and let ODK Validate throw errors when element references are used.
P.S. We should not confuse data types with transformations (that do not change the datatype). With the barcode idea, using a new data type would be the way to go, I think.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-276113505, or mute the thread https://github.com/notifications/unsubscribe-auth/AIO0HjG2O2wV6zy6_9T2xYJ0BRZmjE65ks5rXhIZgaJpZM4Ltmza .
I think these three are the options currently on the table but please correct me if I haven't characterized them properly! I wasn't sure how multiple transforms would work for the transform option with static values so I went with a comma-separated list. It could also be nested functions but that seems more complicated.
An attribute which lets the user specify the maximum size of the long edge. Example: maxpixels=1024
pros: simple
cons: adding transformations for images or for other media types requires adding more attributes
A transform/transformation attribute analogous to appearance
but which can take a comma-separated ordered list of static values (rather than just one). Example: transform="maxpixels(1024),grayscale,jpgcompression(80)"
pros: it's flexible without sacrificing simplicity
cons: allowing only static values and not XPath expressions could be confusing to users (although I think using a comma-separated list makes that easier to reason about? Do most form designers even understand what XPath is?)
A transform/transformation attribute analogous to appearance
which allows for nested XPath functions that take in binary values, transform them, and return binary values. Example: transform="jpgcompression(grayscale(maxpixels(., 1024)), 80)"
pros: it's flexible and kind of complies to an existing standard
cons: it requires some tortured extensions to XPath and those nested functions aren't very easy to read.
The way that I've written these out, 2 seems a lot more reasonable and I think it's what @chrislrobert and @MartijnR were both getting at in their last messages. I don't see any real advantages to 3.
Thanks! Great to hear about all the current and future considerations I'd have never thought of myself.
My vote is for approach 1. I think the advantage of its simplicity and the advantage of its complete XFormsiness makes up for possibly needing multiple attributes in the future. No new parser required to read the value(s). (On the XLSForm side we can still do whatever we want.)
max-pixels
or maxpixels
both fine with me. Preference for former which is also what we ended up with here.
<bind ref="/path/to/node" orx:max-pixels="1024" type="binary"/>
@meletis @chrislrobert Any thoughts on this? It'd be great to move forward on this good idea!
So sorry for the long delay. I can easily be talked into a simple "max-pixels=x" attribute (option 1 with the hyphen added for consistency), but I could also very easily be talked into a more generic "transform" attribute that currently supports "max-pixels(x)" but is spec'd to support a comma-separated list of potential transformations (since indeed, it seems likely we'll want to add others!).
Would it make sense to talk for a brief moment about the XLSForms side of things? Option 2 would lend itself to, maybe, a single new column ("transform"?) whereas Option 1 might lead us down a path of having lots of transformation columns. If that were correct, I would further lead toward Option 2 since I have always always always hated the idea that every single attribute ends up with its own column (so that often you have entire columns that only rarely apply to rows). I understand that we're discussing the XForms side here, but as you all know I am a big fan for designing with XLSForms in mind...
Apologies again about the long delay.
Agree on the single column, comma approach.
I commented on this earlier but I don't get why we are trying to define our own approach for resizing images.
The standard I've seen across a number of libraries looks like this:
convert input.jpg -resize 300 output.jpg
Full read here:
Here is a good article for reference:
https://www.smashingmagazine.com/2015/06/efficient-image-resizing-with-imagemagick/
So I'd propose something like
resize=640,quality=80%
If we want a single function
convert=640,80 (max pixel = 640, 80% quality)
convert=640x480 (if you want to define specific pixels to fit within, sometimes this is helpful).
Note: this includes adding a quality property which I think is important. Default is usually 85% but sometimes you'll want to go down to 60-70% to save on bandwidth depending or higher if you want to do OCR.
On Sun, Mar 12, 2017 at 8:02 PM, chrislrobert notifications@github.com wrote:
So sorry for the long delay. I can easily be talked into a simple "max-pixels=x" attribute (option 1 with the hyphen added for consistency), but I could also very easily be talked into a more generic "transform" attribute that currently supports "max-pixels(x)" but is spec'd to support a comma-separated list of potential transformations (since indeed, it seems likely we'll want to add others!).
Would it make sense to talk for a brief moment about the XLSForms side of things? Option 2 would lend itself to, maybe, a single new column ("transform"?) whereas Option 1 might lead us down a path of having lots of transformation columns. If that were correct, I would further lead toward Option 2 since I have always always always hated the idea that every single attribute ends up with its own column (so that often you have entire columns that only rarely apply to rows). I understand that we're discussing the XForms side here, but as you all know I am a big fan for designing with XLSForms in mind...
Apologies again about the long delay.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-285958174, or mute the thread https://github.com/notifications/unsubscribe-auth/AADiQZaNrzb98HIUdM_aFD58nnf0DkFmks5rlCUvgaJpZM4Ltmza .
@mberg I agree with trying to match XForms, XPath and other related standards when it comes to naming conventions but I'm not sure I understand the value of borrowing names from unrelated tools? I certainly agree that looking at them can be helpful to figure out a minimal list of functions or parameters needed but I don't think we should feel limited by their names if we can come up with something that more accurately represents what we are implementing! I believe the resize
from ImageMagick does what we're proposing for max-pixels
but the name doesn't make that obvious.
@MartijnR Does option 2 strike you as less XForms-y because it's not just fixed strings but a combination of fixed strings with parameters?
I do agree that on the XLSForms side it would be much better to have a single column for transforms. But perhaps that doesn't have to affect the XForms design? I imagine it wouldn't be a big deal to break up the contents of a cell into separate attributes.
I'm just suggesting we follow the conventions laid out by the popular libraries that do this.
Regardless, I'm not wild about max-pixels as it only allows us to set the width based on pixels vs doing something percentage based ie)
resize=640 or resize=50%
Would we need a different command if we want to set image quality too?
On Tue, Mar 14, 2017 at 9:32 PM, Hélène Martin notifications@github.com wrote:
@mberg https://github.com/mberg I agree with trying to match XForms, XPath and other related standards when it comes to naming conventions but I'm not sure I understand the value of borrowing names from unrelated tools? I certainly agree that looking at them can be helpful to figure out a minimal list of functions or parameters needed but I don't think we should feel limited by their names if we can come up with something that more accurately represents what we are implementing! I believe the resize from ImageMagick does what we're proposing for max-pixels but the name doesn't make that obvious.
@MartijnR https://github.com/MartijnR Does option 2 strike you as less XForms-y because it's not just fixed strings but a combination of fixed strings with parameters?
I do agree that on the XLSForms side it would be much better to have a single column for transforms. But perhaps that doesn't have to affect the XForms design? I imagine it wouldn't be a big deal to break up the contents of a cell into separate attributes.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-286517691, or mute the thread https://github.com/notifications/unsubscribe-auth/AADiQX63LFEZ4wQEGPp7Pj2URLCN-p-kks5rlt0tgaJpZM4Ltmza .
@MartijnR Does option 2 strike you as less XForms-y because it's not just fixed strings but a combination of fixed strings with parameters?
Yes, option 2 basically requires parsing a new "language" in the client (in addition to XPath, and for some clients also a special CSV query language ;) (not in the spec, #88)). That's what I don't like about it. XForms has 2 types of values: XPath expressions and fixed strings (including space-separated lists of string values).
Therefore on the XForms side, I'd prefer a different attribute for each transformation type (though as a compromise, maybe a orx:resize
attribute could have values "620px" and "80%" if we have to support both for resizing (which would be too bad).
On the XLSForm side it also seems to me much better to have one "transform" column. However, I think that's perfectly possible (I don't care about adding parsing logic there to generate the different attributes in the output - make it as user-friendly as possible there). So it's just moving the client parsing logic to Pyxform instead. This seems in line with XForms being our actual form format and XLSForm being a (important) preprocessor format.
Okay, well surely there are more than enough chefs in the kitchen, but I'll just make a few quick points here:
I don't know how or when anybody could ever practically put anything relative in a form definition (like "resize to 50%") -- because you will always have forms deployed on different devices with different settings (whether intentional or not), and it would be too hard to imagine a relative rule that would do the right thing under all circumstances.
For the same reason, I have a hard time seeing a fixed pixel size being useful: would you ever want to scale the size up rather than down?
The max-pixels approach (no "quality" concept that depends on image format, no relative sizing) is admittedly more narrow and limited in nature. It's focused on a particular use case. Maybe that's a bad thing. But if you're going to design for a bunch of other use cases, I'd just suggest making sure that they're real use cases users would have in the field.
On Tue, Mar 14, 2017 at 5:45 PM, Martijn van de Rijdt < notifications@github.com> wrote:
@MartijnR https://github.com/MartijnR Does option 2 strike you as less XForms-y because it's not just fixed strings but a combination of fixed strings with parameters?
Yes, option 2 basically requires parsing a new "language" in the client (in addition to XPath, and for some clients also a special CSV query language ;) (not in the spec, #88 https://github.com/opendatakit/xforms-spec/issues/88)). That's what I don't like about it. XForms has 2 types of values: XPath expressions or fixed strings.
Therefore on the XForms side, I'd prefer a different attribute for each transformation type (though as a compromise, maybe a orx:resize attribute could have values "620px" and "80%" if we have to support both for resizing (which would be too bad).
On the XLSForm side it also seems to me much better to have one "transform" column. However, I think that's possible (I don't care about adding parsing logic there to generate the different attributes - make it as user-friendly as possible there). So it's just moving the client parsing logic to Pyxform instead.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-286570831, or mute the thread https://github.com/notifications/unsubscribe-auth/AIO0Huw79ObXjJOwEDHvC5015DZ_2bUKks5rlwprgaJpZM4Ltmza .
Great points, @chrislrobert. Given all this discussion, I'm in favor of adding a max-pixels
custom bind attribute that takes an integer value to the ODK XForms spec and then taking ourselves over to the pyxform repo to come up with the best user experience there.
We could then consider adding a quality attribute in a similar way. Perhaps it could be called jpg-quality
and only get applied if the image taken or selected happens to be a jpg? I believe that quality changes are cumulative so if the image is already at 50% quality and then you ask it to be attached at 50% quality in the form, it may look a lot worse than expected.
So... how do we know when we've made a decision? 😆 It looks like we are mostly in agreement. One last chance for @mberg to try to convince us otherwise? @mberg how would you vote for adding a max-pixels
attribute that specifies the maximum length of the long edge of the corresponding image?
Something to bear in mind, the 'maximum' image size - however that is eventually defined - is something that at the end of the day MUST be checked by the server, which SHOULD reject the form submission if the relevant image exceeds the specified metric(s) [and I'm not shouting here, just trying to use formal RFC requirements terminology: MUST, SHOULD, MAY, etc]. So in that sense, what is in the form specification document, eg a max-pixels
binding attribute, is really just serving as a hint to the client indicating what will be accepted. There is absolutely nothing preventing me bypassing ODK Collect and submitting a record via Aggregate's submission API using, say, curl with whatever images size I want; its ultimately up to the server to enforce it.
For this reason a relative metric, be it "resize=50%" or "quality=80%" really doesn't make much sense to the server, which doesn't really know (or arguably care) what maximum pixel size, or quality, or whatever is technically possible from the hardware on the client device, rather it is more "This is what I'll accept!".
However it is defined, it must be something the server can formally measure so as to make a predictable determination as to whether to accept or reject. Remember, you may tell me what you want, but you cant stop me trying to send you anything I feel like - its up to you to reject it! :-)
Good point. But if form data is encrypted, then the server cannot see the actual size of the image. So enforcement on the server is a non-starter for encrypted forms. Because we ourselves design everything for the encrypted-form case, we'd advocate for a more client-centric approach to enforcement of this kind of thing. But I totally get why the suggestion otherwise makes sense.
On Tue, Apr 25, 2017 at 7:02 AM, Dr. Gareth S. Bestor < notifications@github.com> wrote:
Something to bear in mind, the 'maximum' image size - however that is eventually defined - is something that at the end of the day MUST be checked by the server, which SHOULD reject the form submission if the relevant image exceeds the specified metric(s) [and I'm not shouting here, just trying to use formal RFC requirements terminology: MUST, SHOULD, MAY, etc]. So in that sense, what is in the form specification document, eg a binding attribute, is really just serving as a hint to the client indicating what will be accepted. There is absolutely nothing preventing me bypassing ODK Collect and submitting a record via Aggregate's submission API using, say, curl with whatever images size I want; its ultimately up to the server to enforce it.
For this reason a relative metric, be it "resize=50%" or "quality=80%" really doesn't make much sense to the server, which doesn't really know (or arguably care) what maximum pixel size, or quality, or whatever is technically possible from the hardware on the client device, rather it is more "This is what I'll accept!".
However it is defined, it must be something the server can formally measure so as to make a predictable determination as to whether to accept or reject. Remember, you may tell me what you want, but you cant stop me trying to send you anything I feel like - its up to you to reject it! :-)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-296995496, or mute the thread https://github.com/notifications/unsubscribe-auth/AIO0Hn4ntjbpaVM8xZjtGazQVUJTIp8aks5rzdKzgaJpZM4Ltmza .
@tiritea Thanks, those are great ideas to keep in mind. I think we're in agreement that relative metrics don't make a lot of sense in this context.
We'd like to get this in the May release of Collect/JavaRosa. Is there enough agreement to introduce it as orx:max-pixels
or should we use a different namespace?
If the server cannot decrypt uploaded files then - at some level of your stack - you may need to look into enforcing a filesize limitation. This needn't be explicit in the spec - just well documented somewhere in your particular application - and can be independent of or in addition to an (explicit) max-pixels
or equivalent.
A server with an open upload (aka submission) API must perform some type of enforcement, otherwise you leave yourself open to (malicious?) users swamping and taking down your server. The general rule of thumb in client-server arrangements is "never trust the client"... [which is why, for example, user authentication MUST ultimately be done on the server].
Yes, sure, agreed: the server must authenticate and must protect disk/database space. I just wouldn't know how to translate a max-pixels directive into a parallel encrypted-file-size limitation. At the very least, the relationship between pixels and file size must depend on the image format and nature of the image (e.g., compressibility).
On Tue, Apr 25, 2017 at 5:13 PM, Dr. Gareth S. Bestor < notifications@github.com> wrote:
If the server cannot decrypt uploaded files then - at some level of your stack - you may need to look into enforcing a filesize limitation. This needn't be explicit in the spec - just well documented somewhere in your particular application - and can be independent of or in addition to an (explicit) max-pixels or equivalent.
A server with an open upload (aka submission) API must perform some type of enforcement, otherwise you leave yourself open to (malicious?) users swamping and taking down your server. The general rule of thumb in client-server arrangements is "never trust the client"... [which is why, for example, user authentication MUST ultimately be done on the server].
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opendatakit/xforms-spec/issues/79#issuecomment-297166795, or mute the thread https://github.com/notifications/unsubscribe-auth/AIO0HuaZEqWnxNL37gdCPzyacmu8B-Cnks5rzmH_gaJpZM4Ltmza .
A max-pixels
will - if actually administered by the client - for the most part stop excessively large images being uploaded. But independent of this there's nothing preventing the server implementing its own internal filesize check (in addition?) and simply throwing a 413 Payload-Too-Large back if any file is too big (or it has simply run out of space). And there's nothing saying the server is required to advertise this (!), but it'd be polite if such a hard limit was reasonably greater than what its max-pixels hint suggests, which I think is what you are asking.
I think my point is anything in the XForm is really just a hint to the client as to what will be accepted (max-pixels
, image-type, etc). Actual enforcement still has to happen on the server because nobody can stop me putting whatever I want in an HTTP payload sent to you. And if there's no enforcement, sooner or later someone's going to take you down...
There is the X-OpenRosa-Accept-Content-Length
header that is server-determined and independent from this feature: https://bitbucket.org/javarosa/javarosa/wiki/FormSubmissionAPI. Clients should currently use this published value to stop users from uploading files that are larger than this (technically files that would create submission batches that are larger than that value).
Good to bring that up as the interaction with that and this new feature is important to take into account when implementing the new max-pixels feature.
Is there enough agreement to introduce this as orx:max-pixels
or should we use an ODK Collect specific namespace? @MartijnR
Yes, I believe so! (so orx:max-pixels
)
Hi everybody,
We would like to post a proposal for an extension of the XForm spec.
I'm quoting @chrislrobert's words:
We filed the issue here because we think that we should address this issue in a collaborative way, by discussing design approaches with the community.