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

dadd does not properly validate the time zone #151

Open
rdiez opened this issue Apr 16, 2023 · 6 comments
Open

dadd does not properly validate the time zone #151

rdiez opened this issue Apr 16, 2023 · 6 comments

Comments

@rdiez
Copy link

rdiez commented Apr 16, 2023

The following command "succeeds" without any hint about the erroneous time zone:

dateutils.dadd --zone=NonExistingTimeZone now +2h

The same applies to option "--from-zone".

Such a lack of validation is generally bad practice, but this is especially risky, because the time zone designations may vary from system to system. Most time zone designations are pretty stable and will usually work, but one day one will fail, and that error will probably go unnoticed.

This problem was mentioned in passing in #97.

@hroptatyr
Copy link
Owner

Hi,
thank you for the report. A proposed fix is in a844d56.

@rdiez
Copy link
Author

rdiez commented Apr 18, 2023

First of all, thanks for your effort.

I wouldn't warn though, I would fail. Otherwise, automated scripts will never realise that the date calculations are wrong. Even human users may be reading the result at the end and miss any warnings above.

If you only want to warn, I would document that, in order to assure reliability, the caller should check that nothing gets written to stderr. Because warnings go to stderr, don't they?

Another alternative would be adding a command-line option named "--strict", "--reliable" or "--consider-warnings-as-failures".

hroptatyr added a commit that referenced this issue Apr 20, 2023
hroptatyr added a commit that referenced this issue Apr 20, 2023
* bug/151:
  fix, PEBKAC, tzmaps need lexicographic ordering
  test, adjust regression case to use tzmaps from local test directory
  hygiene, make illegible zone names fatal errors
  Revert "hygiene, warn about unsuccessful lookups of zones or zone associations, addresses bug #151"
@hroptatyr
Copy link
Owner

No you're right, it's no good having low-level code interact with the user somehow. 461a944 lifts the check and handling into the tools, and makes an illegal zone a fatal error.

@rdiez
Copy link
Author

rdiez commented Apr 20, 2023

The new patch seems fine to me.

One minor thing though: I wouldn't use the backtick character ` around %s in error strings like:
Error: cannot find zone specified in --from-zone: `%s'
I would use the standard ASCII apostrophe ' on both sides of %s .

@hroptatyr
Copy link
Owner

Errors of the form `%s' are needed to allow locales to pick the right quoting character.
Currently, dateutils isn't gettextised but I want to keep this option open.

@rdiez
Copy link
Author

rdiez commented Apr 28, 2023

OK, then the patch is fine!

GerHobbelt added a commit to GerHobbelt/dateutils that referenced this issue May 10, 2024
Summary: v0.4.11 of dateutils
Keywords: v0.4.11

This is dateutils v0.4.11.

This is a bugfix release.

Bugfixes:
- be strict about inputs in datetest --isvalid (hroptatyr#146)
- build on Macs again (hroptatyr#107)
- make illegible zone names fatal errors (hroptatyr#151)
- be strict in datetest --isvalid when inputs have been specified (hroptatyr#146)
- fix issue with negative days remaining after adding months in datediff (hroptatyr#153)

See info page examples and/or README.

# -----BEGIN PGP SIGNATURE-----
# Version: GnuPG v2
#
# iEYEABECAAYFAmWyMssACgkQlMmhrILJOQ4F0wCfbiWQfuERz79Mq+BNlyOw0tL6
# RkoAoItx9dPdr/1inKBukpq2McZK2QrF
# =upNa
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu Jan 25 11:07:07 2024 WEST
# gpg:                using DSA key 94C9A1AC82C9390E
# gpg: Can't check signature: No public key
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

2 participants