Accept and display profile images of any format.

yrammos commented

2020-06-13 12:37:43 +02:00

(Migrated from github.com)

glewlwyd currently accepts only profile images of a single hard-wired format (JPEG by default). Uploading a different file format results in an invalid Base64 URI. Although it should be possible to detect the image type and adjust the Base64 URI accordingly, in this PR I am opting for the quickest possible workaround, namely an accept="image/jpeg" attribute for the image <input> tag. This solution is a proof of concept, so to say, and has two drawbacks: it does not accept X-PNG's or BMP's, and it remains fragile by not validating the image data on the server side. It's just a start to raise the issue. In a stronger solution, I think that the picture pattern MIME type would need to be modified to "image/* in config.json, the image upload routines modified to prepend the image type to the image URI, and the file contents examined/checksummed server-side to prevent invalid data from being stored.

glewlwyd currently accepts only profile images of a single hard-wired format (JPEG by default). Uploading a different file format results in an invalid Base64 URI. Although it should be possible to detect the image type and adjust the Base64 URI accordingly, in this PR I am opting for the quickest possible workaround, namely an `accept="image/jpeg"` attribute for the image `<input>` tag. This solution is a proof of concept, so to say, and has two drawbacks: it does not accept X-PNG's or BMP's, and it remains fragile by not validating the image data on the server side. It's just a start to raise the issue. In a stronger solution, I think that the `picture` pattern MIME type would need to be modified to `"image/*` in `config.json`, the image upload routines modified to prepend the image type to the image URI, and the file contents examined/checksummed server-side to prevent invalid data from being stored.

yrammos commented

2020-06-13 12:40:59 +02:00

(Migrated from github.com)

Meanwhile, @babelouest I'd be grateful if you could provide a MySQL script to safely clean up invalid image data from my user records. My g_user_property database table is full of dashes (-), which I presume is a sign of corruption.

Meanwhile, @babelouest I'd be grateful if you could provide a MySQL script to safely clean up invalid image data from my user records. My `g_user_property` database table is full of dashes (`-`), which I presume is a sign of corruption.

yrammos commented

2020-06-13 14:52:54 +02:00

(Migrated from github.com)

UPDATE: According to RFC 2046 (Section 4.2):

Unrecognized subtypes of "image" should at a miniumum be treated as "application/octet-stream". Implementations may optionally elect to pass subtypes of "image" that they do not specifically recognize to a secure and robust general-purpose image viewing application, if such an application is available.

Therefore I'm revising (rebase-squashing) this PR to accept="image/*". This allows the form to accept files of any "image" extension as profile photo. Setting the picture pattern type to the generic image/image in config.json should then be enough, because browsers are smart enough to infer the image format from its Base64 encoding (in RFC wording, browsers are "robust general-purpose image viewing applications"). Validating the data server-side would still be nice.

**UPDATE**: According to [RFC 2046 (Section 4.2)](https://tools.ietf.org/html/rfc2046#section-4.2): > Unrecognized subtypes of "image" should at a miniumum be treated as "application/octet-stream". Implementations may optionally elect to pass subtypes of "image" that they do not specifically recognize to a secure and robust general-purpose image viewing application, if such an application is available. Therefore I'm revising (rebase-squashing) this PR to `accept="image/*"`. This allows the form to accept files of any "image" extension as profile photo. Setting the `picture` pattern type to the generic `image/image` in `config.json` should then be enough, because browsers are smart enough to infer the image format from its Base64 encoding (in RFC wording, browsers are "robust general-purpose image viewing applications"). Validating the data server-side would still be nice.

babelouest commented

2020-06-13 15:56:11 +02:00

(Migrated from github.com)

Thanks again @yrammos ,

I think similar changes should be done in the profile page too, maybe in the login page as well: https://github.com/babelouest/glewlwyd/blob/yrammos-edit-user-modal-bugfix/webapp-src/src/Profile/User.js

Thanks again @yrammos , I think similar changes should be done in the profile page too, maybe in the login page as well: https://github.com/babelouest/glewlwyd/blob/yrammos-edit-user-modal-bugfix/webapp-src/src/Profile/User.js

babelouest commented

2020-06-13 16:11:12 +02:00

(Migrated from github.com)

Validating the image data on the server side is difficult, it would imply using an image library like imageMagick or something else, but mostly it would open a pandora box of complication on the server side which I don't want to maintain.
Worst-case scenario is if the user uploads garbage data as image, the result will be a non displayed image and I'm fine with that. If the user tries to find bugs to exploit, she/he wont' read the error messages, if the user uploaded a garbage image by mistake, she/he will reupload a good one.

To cleanup the images in the database, you can run the following SQL query:

delete from g_user_property where gup_name='picture' and gu_id in (select gu_id from g_user where gu_username='{USER}');

Where {USER} is the username with crappy images

Validating the image data on the server side is difficult, it would imply using an image library like imageMagick or something else, but mostly it would open a pandora box of complication on the server side which I don't want to maintain. Worst-case scenario is if the user uploads garbage data as image, the result will be a non displayed image and I'm fine with that. If the user tries to find bugs to exploit, she/he wont' read the error messages, if the user uploaded a garbage image by mistake, she/he will reupload a good one. To cleanup the images in the database, you can run the following SQL query: ```SQL delete from g_user_property where gup_name='picture' and gu_id in (select gu_id from g_user where gu_username='{USER}'); ``` Where `{USER}` is the username with crappy images

yrammos commented

2020-06-13 16:12:52 +02:00

(Migrated from github.com)

if the user uploaded a garbage image by mistake, she/he will reupload a good one.

Agreed, @babelouest—and many thanks for the query.—with the caveat that re-uploading seems impossible if garbage has already been uploaded (I tried it). This is why I asked for the query. But I'm personally OK with considering this a lunatic-fringe case (<.001 chance).

> if the user uploaded a garbage image by mistake, she/he will reupload a good one. Agreed, @babelouest—and many thanks for the query.~—with the caveat that re-uploading seems impossible if garbage has already been uploaded (I tried it). This is why I asked for the query. But I'm personally OK with considering this a lunatic-fringe case (<.001 chance).~

yrammos commented

2020-06-13 16:53:59 +02:00

(Migrated from github.com)

False alarm. Please disregard last message.

babelouest commented

2020-06-13 17:35:07 +02:00

(Migrated from github.com)

Yeah, I was gonna talk about that, you can remove an image by clicking on it, but it's not obvious at all, I'll add a X or a trash icon to make it more ergonomic

babelouest commented

2020-06-13 19:09:35 +02:00

(Migrated from github.com)

@yrammos , I've added a trash icon on top right of the image like this:

@yrammos , I've added a trash icon on top right of the image like this: ![Capture d’écran du 2020-06-13 13-07-51](https://user-images.githubusercontent.com/3966617/84574789-ecdeec00-ad76-11ea-8fab-30c747673c64.png)

🎉 1 🚀 1

Rows
Columns

Accept and display profile images of any format. #135