Opened 5 years ago
Last modified 20 months ago
#48204 new defect (bug)
wp_exif_date2ts throws notice on empty date
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | minor | Version: | 5.2.3 |
Component: | Date/Time | Keywords: | has-patch |
Focuses: | Cc: |
Description
When an image has the EXIF field DateTimeDigitized defined, but set to an empty timestamp, like
: : : :
a PHP notice is thrown during upload.
I don't know how common this is, but I've seen it on several family photos that were scanned by my aunt. The EXIF info mentions both HP Scanjet 4370, FinePixViewer Ver.4.2, and FUJIFILM.
Anyway, it can be pre-empted with this between the two calls to explode():
if ( ':' === $date ) { return ''; }
I've attached a sample file to replicate the problem.
Attachments (1)
Change History (12)
#1
@
5 years ago
- Keywords reporter-feedback added
Thanks @nosilver4u, can you share actual message from the PHP Notice you experienced here?
#2
@
5 years ago
oh sure, it's this:
Notice: Undefined offset: 2 in /var/www/html/wordpress/wp-admin/includes/image.php on line 637
FYI, I'm actually running 5.3-beta2-46373 on my dev site at the moment.
#3
@
5 years ago
just tested on 5.2.3, and don't see the error, so perhaps that actually is a new notice in 5.3
#4
@
5 years ago
- Keywords needs-patch added; reporter-feedback removed
- Milestone changed from Awaiting Review to Future Release
Perfect. Thank you. Tested and am seeing the same error. The problem here is the assumption being made in this function is that there will only be one whitespace character in the string, because we're expecting a string formatted like yyyy:MM:dd HH:mm:ss
, so the first explode()
statement should split the date from the time, and the second explode
should return the date parts. If the original string includes any additional whitespaces, as is the case here, those assumptions are incorrect.
As a sidenote, the docblock for wp_exif_date2ts()
says that this function will always return an int
but it could also return false
since it's just passing along the results of strtotime()
— see: https://www.php.net/manual/en/function.strtotime.php
#5
@
5 years ago
Indeed, and I see why the notice was not displayed before, version 5.2.3 uses the error suppression operator:
@list( $date, $time ) = explode( ' ', trim( $str ) ); @list( $y, $m, $d ) = explode( ':', $date );
#7
@
5 years ago
Best next steps here would be to add a unit test for wp_exif_date2ts()
(if none exists already) that ensures that strings that match our expected format return the expected value and also that strings that don't match our expected format return a either a false
value or null
, then update the internals of that function so that both tests pass.
This ticket was mentioned in PR #2025 on WordPress/wordpress-develop by xaqrox.
3 years ago
#10
- Keywords has-patch added; needs-patch removed
I believe @dshanke's PR Wordpress/wordpress-develop#495 also fixes https://core.trac.wordpress.org/ticket/48204 but there is a syntax error.
Trac ticket: https://core.trac.wordpress.org/ticket/48204
### Original PR description:
This patch removes uses of strtotime from image metadata functions, replacing them with the DateTime class. It introduces a new wp_exif_datetime function that can be used for this. wp_exif_date2ts if now a wrapper around it that generates a timestamp for backcompat only. It replaces the use of the old function in the read metadata functionality and to ensure timezone is respected, stores as a new property the full datetime with timezone.
Trac ticket: https://core.trac.wordpress.org/ticket/49413
#11
@
20 months ago
I too was in the process of importing data from another site and some image files were showing warning errors.
The contents of the DateTimeDigitized item in the corresponding file had the following three patterns.
1. string(1) " " 2. string(19) " : : : : " 3. string(19) "2017/07/02 17:26:46"
I was able to confirm the first and second patterns in the specifications, but not the third.
As for the first, there may be a way to change the side that calls the wp_exif_date2ts().
if ( empty( $meta['created_timestamp'] ) && ! empty( trim( $exif['DateTimeDigitized'] ) ) ) { $meta['created_timestamp'] = wp_exif_date2ts( $exif['DateTimeDigitized'] ); }
In the above, the DateTimeDigitized item is trimmed before determining if it is empty.
file with "blank" DateTimeDigitized