Skip to content
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

Date handling when simplifying date strings without a timezone seems incorrect #14

Open
dremonkey opened this issue Mar 23, 2016 · 9 comments

Comments

@dremonkey
Copy link

First off... thanks for the library. Of the ones that I have seen it is by far the most complete.

However, it seems like your logic when simplifying a date string may be wrong. As far as I can tell you are treating a date string without the an explicit timezone as UTC time when according to ISO standards it should be treated as local time 1.

If you agree that this is a problem I can open a pull request with a fix...

@dremonkey dremonkey changed the title Date handling in simply seems wrong Date handling when simplifying date strings without a timezone seems incorrect Mar 23, 2016
@dremonkey
Copy link
Author

Actually after giving this a bit more thought... my suggestion would be to remove the simplify option completely and focus only on parsing. Let a more complete library like moment.js handle an transformations on the data. Thoughts?

@rprieto
Copy link

rprieto commented Jan 26, 2017

Hi, I second that: thanks for this awesome EXIF parser! I think I have similar issues with DateTimeOriginal and timezones. For a concrete example, I have a photo called 2017-01-04 11.29.12.jpg (Dropbox naming convention, I did take it at 11:29am). If I call exiftool directly, it confirms:

Date/Time Original: 2017:01:04 11:29:12   # matches the file name

When I use exif-parser, I get:

exifparser.create(fileContents).parse().tags.DateTimeOriginal)
// returns 1483529352

new Date(1483529352 * 1000)
// Wed Jan 04 2017 22:29:12 GMT+1100 (AEDT)

So it looks like it assumed GMT and added 11 hours for AEDT which is my local timezone. I started noticing because I just added the "create albums by date" functionality to thumbsup (which uses exif-parser as the core parser), and noticed photos from New Years Eve appearing in the "January" album 😄).

@bwindels
Copy link
Owner

Wow, this is an old one. Sorry I missed this for so long. Great catch about the default time zone. Also the parsing for explicit time zones is quite incomplete. Briefly looking at the spec; optional colons between hours and minutes of the offset are not handled and the Z postfix for UTC is not handled at all.

I would not remove simplifying EXIF values completely, as
a) you can disable it
b) numeric EXIF values are expressed in unique ways (fractional, degrees) that make little sense to consume as is.
c) I don't want to require a third-party library like moment.js just to parse a date

Splitting up in two libraries would be too much overhead as the release process is already somewhat cumbersome.

I would be very happy with a PR for fixing the default time zone to local time, or a setting to just disable simplifying dates :)

@bwindels
Copy link
Owner

Could use this for the parsing: https://www.npmjs.com/package/iso8601-convert

@edemaine
Copy link

edemaine commented Jul 26, 2018

Just coming to this package for the first time, and ran into this issue. I assume it remains unresolved?

A few notes from the EXIF 2.2 spec:

  • DateTimes have the format YYYY:MM:DD HH:MM:SS of exactly 19 characters (plus null terminator). The colons are not optional, and timezones cannot be specified.
  • Each Y/M/D/H/M/S character can instead be a space. That would wreck havoc with the split(' ') currently in the code. (I've never seen this in practice, though, and the regex in another package https://www.npmjs.com/package/exif-date is similarly broken.)

I'm not sure it would be great to return the local time, when this library can be used server side and the client's notion of timezone may differ. I can see a couple of options:

  1. Leave the date as an EXIF string. Not ideal, but then the user can do what they want.
  2. Change the date into an ISO8601 string, so it's easy to parse via e.g. moment (by default with local timezone, or you could easy add a timezone offset to the end)

Personally I think Option 2 is a bit better. Thoughts?

edemaine added a commit to edemaine/exif-parser that referenced this issue Jul 26, 2018
Fix bwindels#14 by not converting to local or UTC date, instead slightly
modifying DateTime strings into easy-to-parse ISO 8601 format.

When a DateTime string follows the EXIF spec, we map some colons to
dashes to conform to ISO 8601.  The other supported format was
ISO 8601, so we don't do anything in this case.
edemaine added a commit to edemaine/exif-parser that referenced this issue Jul 26, 2018
Fix bwindels#14 by not converting to local or UTC date, instead slightly
modifying DateTime strings into easy-to-parse ISO 8601 format.

When a DateTime string follows the EXIF spec, we map some colons to
dashes to conform to ISO 8601.  The other supported format was
ISO 8601, so we don't do anything in this case.
@bwindels
Copy link
Owner

Yeah, parsing dates with a timezone needs to be improved. I'd rather do it with a very light dependency as mentioned before, or none at all. moment.js is 16kb minified, too big to justify for this problem IMO.

Apart from that, one thing mentioned in earlier comments is that some tools assume that a date without a timezone is assumed to be in the local timezone. An option to set the default timezone could make sense. I'll do it when I get to it, unless anyone beats to me making a PR :)

@lqzhgood
Copy link

The time zone is wrong

@bwindels
Copy link
Owner

The time zone is wrong

Care to elaborate? Can you share the date string in the file you see the issue with?

@lqzhgood
Copy link

The time zone is wrong

Care to elaborate? Can you share the date string in the file you see the issue with?

like #14 (comment)

I test .Jpg Exif is local time zone not 0 time zone(GMT). --> exif.DateTimeOriginal

Relevant links found
https://exiftool.org/forum/index.php?topic=1886.0
https://community.adobe.com/t5/lightroom-classic/how-to-change-the-timezone-in-photo-metadata/td-p/6429833?page=1

by the way

I test .mov is 0 time zone(GMT) not local time zone~ --> exif.createDate
Exif specification It's too messy .....

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

No branches or pull requests

5 participants