Skip to content

Remove dependency on Exif parsing library. #2947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ dependencies {
implementation 'com.jakewharton.rxbinding2:rxbinding-appcompat-v7:2.1.1'
implementation 'com.jakewharton.rxbinding2:rxbinding-design:2.1.1'
implementation 'com.facebook.fresco:fresco:1.13.0'
implementation 'com.drewnoakes:metadata-extractor:2.11.0'
implementation 'com.dmitrybrant:wikimedia-android-data-client:0.0.18'
implementation 'org.apache.commons:commons-lang3:3.8.1'

Expand Down Expand Up @@ -95,8 +94,9 @@ dependencies {
implementation "androidx.cardview:cardview:1.0.0"
implementation 'androidx.constraintlayout:constraintlayout:1.1.3'
implementation "androidx.exifinterface:exifinterface:1.0.0"
//metadata extractor
implementation 'com.drewnoakes:metadata-extractor:2.11.0'

//swipe_layout
implementation 'com.daimajia.swipelayout:library:1.2.0@aar'
}

android {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
package fr.free.nrw.commons.filepicker;

import android.annotation.SuppressLint;
import android.content.Context;
import android.database.Cursor;
import android.net.Uri;
import android.os.Parcel;
import android.os.Parcelable;

import androidx.exifinterface.media.ExifInterface;
import androidx.annotation.Nullable;

import com.drew.imaging.ImageMetadataReader;
import com.drew.imaging.ImageProcessingException;
import com.drew.metadata.Metadata;
import com.drew.metadata.exif.ExifSubIFDDirectory;

import java.io.File;
import java.io.IOException;
import java.util.Date;
Expand Down Expand Up @@ -125,16 +122,13 @@ private DateTimeWithSource getFileCreatedDateFromCP(Context context) {
* @return
*/
private DateTimeWithSource getDateTimeFromExif() {
Metadata metadata;
try {
metadata = ImageMetadataReader.readMetadata(file);
ExifSubIFDDirectory directory = metadata.getFirstDirectoryOfType(ExifSubIFDDirectory.class);
if (directory!=null && directory.containsTag(ExifSubIFDDirectory.TAG_DATETIME_ORIGINAL)) {
Date date = directory.getDate(ExifSubIFDDirectory.TAG_DATETIME_ORIGINAL);
ExifInterface exif = new ExifInterface(file.getAbsolutePath());
@SuppressLint("RestrictedApi") long dateTime = exif.getDateTime();
if (dateTime != -1) {
Date date = new Date(dateTime);
return new DateTimeWithSource(date, DateTimeWithSource.EXIF_SOURCE);
}
} catch (ImageProcessingException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
Expand Down
48 changes: 12 additions & 36 deletions app/src/main/java/fr/free/nrw/commons/upload/EXIFReader.java
Original file line number Diff line number Diff line change
@@ -1,60 +1,36 @@
package fr.free.nrw.commons.upload;

import com.drew.imaging.ImageMetadataReader;
import com.drew.imaging.ImageProcessingException;
import com.drew.metadata.Directory;
import com.drew.metadata.Metadata;

import java.io.File;
import java.io.IOException;
import androidx.exifinterface.media.ExifInterface;

import javax.inject.Inject;
import javax.inject.Singleton;

import fr.free.nrw.commons.utils.ImageUtils;
import io.reactivex.Single;
import timber.log.Timber;

/**
* We try to avoid copyright violations in commons app.
* For doing that we read EXIF data using the library metadata-reader
* If an image doesn't have any EXIF Directoris in it's metadata then the image is an
* internet download image(and not the one taken using phone's camera) */

* We try to minimize uploads from the Commons app that might be copyright violations.
* If an image does not have any Exif metadata, then it was likely downloaded from the internet,
* and is probably not an original work by the user. We detect these kinds of images by looking
* for the presence of some basic Exif metadata.
*/
@Singleton
public class EXIFReader {
@Inject
public EXIFReader() {
//Empty
}
/**
* The method takes in path of the image and reads metadata using the library metadata-extractor
* And the checks for the presence of EXIF Directories in metadata object
* */

public Single<Integer> processMetadata(String path) {
Metadata readMetadata = null;
try {
readMetadata = ImageMetadataReader.readMetadata(new File(path));
} catch (ImageProcessingException e) {
Timber.d(e.toString());
} catch (IOException e) {
Timber.d(e.toString());
}
if (readMetadata != null) {
for (Directory directory : readMetadata.getDirectories()) {
// In case of internet downloaded image these three fields are not present
if (directory.getName().equals("Exif IFD0") //Contains information about the device capturing the photo
|| directory.getName().equals("Exif SubIFD") //contains information like date, time and pixels of the image
|| directory.getName().equals("Exif Thumbnail")) //contains information about image thumbnail like compression and reolution
{
Timber.d(directory.getName() + " Contains metadata");
return Single.just(ImageUtils.IMAGE_OK);
}
ExifInterface exif = new ExifInterface(path);
if (exif.getAttribute(ExifInterface.TAG_MAKE) != null
|| exif.getAttribute(ExifInterface.TAG_DATETIME) != null) {
return Single.just(ImageUtils.IMAGE_OK);
}
} catch (Exception e) {
return Single.just(ImageUtils.FILE_NO_EXIF);
}
return Single.just(ImageUtils.FILE_NO_EXIF);
}

}

Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
package fr.free.nrw.commons.upload;

import android.content.Context;
import android.net.Uri;

import org.apache.commons.lang3.StringUtils;

import java.io.IOException;

import javax.inject.Inject;
import javax.inject.Singleton;

Expand All @@ -23,30 +18,24 @@

/**
* Methods for pre-processing images to be uploaded
*//*if (dataInBytes[0] == 70 && dataInBytes[1] == 66 && dataInBytes[2] == 77 && dataInBytes[3] == 68) {
Timber.d("Contains FBMD");
return Single.just(ImageUtils.FILE_FBMD);
}*/
*/
@Singleton
public class ImageProcessingService {
private final FileUtilsWrapper fileUtilsWrapper;
private final ImageUtilsWrapper imageUtilsWrapper;
private final MediaWikiApi mwApi;
private final ReadFBMD readFBMD;
private final EXIFReader EXIFReader;
private final Context context;

@Inject
public ImageProcessingService(FileUtilsWrapper fileUtilsWrapper,
ImageUtilsWrapper imageUtilsWrapper,
MediaWikiApi mwApi, ReadFBMD readFBMD, EXIFReader EXIFReader,
Context context) {
MediaWikiApi mwApi, ReadFBMD readFBMD, EXIFReader EXIFReader) {
this.fileUtilsWrapper = fileUtilsWrapper;
this.imageUtilsWrapper = imageUtilsWrapper;
this.mwApi = mwApi;
this.readFBMD = readFBMD;
this.EXIFReader = EXIFReader;
this.context = context;
}

/**
Expand All @@ -64,12 +53,11 @@ Single<Integer> validateImage(UploadModel.UploadItem uploadItem, boolean checkTi
}
Timber.d("Checking the validity of image");
String filePath = uploadItem.getMediaUri().getPath();
Uri contentUri=uploadItem.getContentUri();
Single<Integer> duplicateImage = checkDuplicateImage(filePath);
Single<Integer> wrongGeoLocation = checkImageGeoLocation(uploadItem.getPlace(), filePath);
Single<Integer> darkImage = checkDarkImage(filePath);
Single<Integer> itemTitle = checkTitle ? validateItemTitle(uploadItem) : Single.just(ImageUtils.IMAGE_OK);
Single<Integer> checkFBMD = checkFBMD(context,contentUri);
Single<Integer> checkFBMD = checkFBMD(filePath);
Single<Integer> checkEXIF = checkEXIF(filePath);

Single<Integer> zipResult = Single.zip(duplicateImage, wrongGeoLocation, darkImage, itemTitle,
Expand All @@ -84,31 +72,21 @@ Single<Integer> validateImage(UploadModel.UploadItem uploadItem, boolean checkTi
}

/**
* Other than the Image quality we need to check that using this Image doesn't violate's facebook's copyright's.
* Whenever a user tries to upload an image that was downloaded from Facebook then we warn the user with a message to stop the upload
* To know whether the Image is downloaded from facebook:
* -We read the metadata of any Image and check for FBMD
* -Facebook downloaded image's contains metadata of the type IPTC
* - From this IPTC metadata we extract a byte array that contains FBMD as it's initials. If the image was downloaded from facebook
* Thus we successfully protect common's from Facebook's copyright violation
* We want to discourage users from uploading images to Commons that were taken from Facebook.
* This attempts to detect whether an image was downloaded from Facebook by heuristically
* searching for metadata that is specific to images that come from Facebook.
*/

public Single<Integer> checkFBMD(Context context,Uri contentUri) {
try {
return readFBMD.processMetadata(context,contentUri);
} catch (IOException e) {
return Single.just(ImageUtils.FILE_FBMD);
}
private Single<Integer> checkFBMD(String filepath) {
return readFBMD.processMetadata(filepath);
}

/**
* To avoid copyright we check for EXIF data in any image.
* Images that are downloaded from internet generally don't have any EXIF data in them
* while images taken via camera or screenshots in phone have EXIF data with them.
* So we check if the image has no EXIF data then we display a warning to the user
* * */

public Single<Integer> checkEXIF(String filepath){
* We try to minimize uploads from the Commons app that might be copyright violations.
* If an image does not have any Exif metadata, then it was likely downloaded from the internet,
* and is probably not an original work by the user. We detect these kinds of images by looking
* for the presence of some basic Exif metadata.
*/
private Single<Integer> checkEXIF(String filepath) {
return EXIFReader.processMetadata(filepath);
}

Expand Down
51 changes: 22 additions & 29 deletions app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
package fr.free.nrw.commons.upload;

import android.content.Context;
import android.net.Uri;

import com.drew.imaging.ImageMetadataReader;
import com.drew.imaging.ImageProcessingException;
import com.drew.metadata.Metadata;
import com.drew.metadata.Tag;
import com.drew.metadata.iptc.IptcDirectory;

import java.io.FileInputStream;
import java.io.IOException;

import javax.inject.Inject;
Expand All @@ -17,37 +9,38 @@
import fr.free.nrw.commons.utils.ImageUtils;
import io.reactivex.Single;

/**
* We want to discourage users from uploading images to Commons that were taken from Facebook.
* This attempts to detect whether an image was downloaded from Facebook by heuristically
* searching for metadata that is specific to images that come from Facebook.
*/
@Singleton
public class ReadFBMD {

@Inject
public ReadFBMD() {

}

public Single<Integer> processMetadata(Context context, Uri contentUri) throws IOException {
Metadata readMetadata = null;
public Single<Integer> processMetadata(String path) {
try {
readMetadata = ImageMetadataReader.readMetadata(context.getContentResolver().openInputStream(contentUri));
} catch (ImageProcessingException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
int psBlockOffset;
int fbmdOffset;

try (FileInputStream fs = new FileInputStream(path)) {
byte[] bytes = new byte[4096];
fs.read(bytes);
fs.close();
String fileStr = new String(bytes);
psBlockOffset = fileStr.indexOf("8BIM");
fbmdOffset = fileStr.indexOf("FBMD");
}

IptcDirectory iptcDirectory = readMetadata != null ? readMetadata.getFirstDirectoryOfType(IptcDirectory.class) : null;
if (iptcDirectory == null) {
return Single.just(ImageUtils.IMAGE_OK);
}
/**
* We parse through all the tags in the IPTC directory if the tagname equals "Special Instructions".
* And the description string starts with FBMD.
* Then the source of image is facebook
* */
for (Tag tag : iptcDirectory.getTags()) {
if (tag.getTagName().equals("Special Instructions") && tag.getDescription().substring(0, 4).equals("FBMD")) {
if (psBlockOffset > 0 && fbmdOffset > 0
&& fbmdOffset > psBlockOffset && fbmdOffset - psBlockOffset < 0x80) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While removing a 656 KB library is great, in my humble opinion I would say that this implementation is much less readable, replacing easy-to-read business logic with byte manipulation and unexplained magic codes such as "8BIM" and 0x80.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some comments/explanations could help. In that case would you still think this is less readable @nicolas-raoul ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbrant is it possible to add some comments here to make it more readable?

return Single.just(ImageUtils.FILE_FBMD);
}
} catch (IOException e) {
e.printStackTrace();
}
return Single.just(ImageUtils.IMAGE_OK);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class u {
.thenReturn(false)
`when`(mwApi!!.fileExistsWithName(ArgumentMatchers.anyString()))
.thenReturn(false)
`when`(readFBMD?.processMetadata(ArgumentMatchers.any(),ArgumentMatchers.any()))
`when`(readFBMD?.processMetadata(ArgumentMatchers.any()))
.thenReturn(Single.just(ImageUtils.IMAGE_OK))
`when`(readEXIF?.processMetadata(ArgumentMatchers.anyString()))
.thenReturn(Single.just(ImageUtils.IMAGE_OK))
Expand Down