Skip to content

Conversation

davidyell
Copy link
Member

When trying to upload an image with incorrect exif data this method will throw a warning. This is because it also validates the Exif data against the spec to make sure it's valid.

exif_read_data() also validates EXIF data tags according to the EXIF specification (» http://exif.org/Exif2-2.PDF, page 20).

You'll get this warning.

Warning (2): exif_read_data(main_vienna_6.jpg): Incorrect APP1 Exif Identifier Code [APP/Plugin/Upload/Model/Behavior/UploadBehavior.php, line 1502]

Images with invalid exif information are still uploaded though, so it doesn't have any functional impact. However if devs are logging errors this will fill up an error log pretty quickly.

Silenced exif_read_data to prevent warnings when images do not have valid exif data.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f3803e6 on davidyell:patch-1 into 14004cf on josegonzalez:master.

@davidyell
Copy link
Member Author

Relates to #221

@davidyell
Copy link
Member Author

What are your thoughts on this @josegonzalez ? Travis is just failing for PHPCS, but is silencing this error really that bad?

It would suck that Travis would fail PHPCS for every run though.

@davidyell davidyell closed this Feb 4, 2015
@davidyell davidyell deleted the patch-1 branch February 4, 2015 11:07
@josegonzalez
Copy link
Member

I think you can do something like NOQA or similar?

@davidyell davidyell restored the patch-1 branch February 4, 2015 15:35
@davidyell
Copy link
Member Author

Didn't mean to remove that branch!

@davidyell davidyell reopened this Feb 4, 2015
@davidyell davidyell self-assigned this Feb 9, 2015
@davidyell
Copy link
Member Author

I haven't been able to find anything on the NOQA thing, but I did find this.

$ phpcs --config-set ignore_warnings_on_exit 1
https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#ignoring-warnings-when-generating-the-exit-code

Not sure if this will impact the project by letting bad code through though on future PR's.

@davidyell davidyell closed this Jun 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants