[WIP] Img exif #3476

2020-01-12T05:49:45+01:00

tuxfoo commented

2020-01-12 05:49:45 +01:00

(Migrated from github.com)

PR Checklist

I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
I have checked that this PR does not introduce a breaking change

PR Type

What kind of change does this PR introduce?

[ x] Bugfix

Fixes

Issue Number: #3435

What is the current behavior?

EXIF data is ignored, this leads to photos been shown with the wrong orientation. This can be a problem as people take photos with their smart phones upside down and sideways.

What is the new behavior?

The orientation of the photo is honoured using EXIF data.

Other information

At the time of writing this PR this patch only works for the preview, I still need to implement this for the thumbnails.
I thought I would submit this now because I was wondering if it was okay to use the blueimp-load-image library.
https://github.com/blueimp/JavaScript-Load-Image

In the future, it might be a good idea if the client could remove such meta data on upload. Especially since it can contain Geo data.

## PR Checklist - [x] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged) - [ ] I have checked that this PR does not introduce a breaking change ## PR Type What kind of change does this PR introduce? - [ x] Bugfix ## Fixes Issue Number: #3435 ## What is the current behavior? EXIF data is ignored, this leads to photos been shown with the wrong orientation. This can be a problem as people take photos with their smart phones upside down and sideways. ## What is the new behavior? The orientation of the photo is honoured using EXIF data. ## Other information At the time of writing this PR this patch only works for the preview, I still need to implement this for the thumbnails. I thought I would submit this now because I was wondering if it was okay to use the `blueimp-load-image` library. https://github.com/blueimp/JavaScript-Load-Image In the future, it might be a good idea if the client could remove such meta data on upload. Especially since it can contain Geo data.

tzarebczan commented

2020-01-13 18:19:38 +01:00

(Migrated from github.com)

Thanks for the PR @tuxfoo ! We'll get a review on this soon. Once it's completed, can we show you some appreciation ?

Thanks for the PR @tuxfoo ! We'll get a review on this soon. Once it's completed, can we show you some [appreciation](https://lbry.com/faq/appreciation) ?

tuxfoo commented

2020-01-19 11:54:17 +01:00

(Migrated from github.com)

Thanks for the PR @tuxfoo ! We'll get a review on this soon. Once it's completed, can we show you some appreciation ?

So it is okay to use the blueimp image library? I will have to change img and background-image to canvas. I can attempt to apply the fix to the thumbnail on the claim-list as well.

> Thanks for the PR @tuxfoo ! We'll get a review on this soon. Once it's completed, can we show you some [appreciation](https://lbry.com/faq/appreciation) ? So it is okay to use the blueimp image library? I will have to change img and background-image to canvas. I can attempt to apply the fix to the thumbnail on the claim-list as well.

neb-b (Migrated from github.com) reviewed 2020-01-20 18:23:26 +01:00

ui/component/viewers/imageViewer.jsx

					
				@ -14,0 +13,4 @@

				      if (img.type === 'error') {

				        console.error('Cannot load image');

				      } else {

				        document.getElementsByClassName('file-render__viewer')[0].appendChild(img);

neb-b (Migrated from github.com) commented

2020-01-20 18:23:25 +01:00

You should move loadPic inside of the ImageViewer component so that it can use refs and dynamically inserting the image, instead of using appendChild

I think it's fine to use this library though.

You should move `loadPic` inside of the `ImageViewer` component so that it can use `refs` and dynamically inserting the image, instead of using `appendChild` I think it's fine to use this library though.

neb-b (Migrated from github.com) reviewed 2020-01-20 18:23:48 +01:00

package.json

					
				@ -44,6 +44,7 @@

				  },

				  "dependencies": {

				    "auto-launch": "^5.0.5",

				    "blueimp-load-image": "^2.26.0",

neb-b (Migrated from github.com) commented

2020-01-20 18:23:48 +01:00

This should be a devDependency

This should be a `devDependency`

kauffj commented

2020-01-27 21:08:01 +01:00

(Migrated from github.com)

@tuxfoo come backkkkkkkkkkkkkk

jsigwart commented

2020-01-27 21:15:22 +01:00

(Migrated from github.com)

How do we keep from malicious EXIF data being uploaded? https://umbrella.cisco.com/blog/2019/07/24/picture-perfect-how-jpg-exif-data-hides-malware/

tuxfoo commented

2020-01-27 21:34:47 +01:00

(Migrated from github.com)

@tuxfoo come backkkkkkkkkkkkkk

Sorry back to work this week. I need read up on using refs.

How do we keep from malicious EXIF data being uploaded? https://umbrella.cisco.com/blog/2019/07/24/picture-perfect-how-jpg-exif-data-hides-malware/

I think metadata of images should be removed on upload(after correcting the orientation).
Malicious metadata is not the only concern, images can contain geo data that could put the uploader at risk.
However, I am not sure if it is possible to prevent people from uploading malicious content via the the command line.

> @tuxfoo come backkkkkkkkkkkkkk Sorry back to work this week. I need read up on using refs. > How do we keep from malicious EXIF data being uploaded? https://umbrella.cisco.com/blog/2019/07/24/picture-perfect-how-jpg-exif-data-hides-malware/ I think metadata of images should be removed on upload(after correcting the orientation). Malicious metadata is not the only concern, images can contain geo data that could put the uploader at risk. However, I am not sure if it is possible to prevent people from uploading malicious content via the the command line.

kauffj commented

2020-02-10 21:10:18 +01:00

(Migrated from github.com)

@tuxfoo would still love to see this, but will probably close PR next week if no progress

malicious EXIF data can be considered out of scope here

@tuxfoo would still love to see this, but will probably close PR next week if no progress malicious EXIF data can be considered out of scope here