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

Throw IllegalArgumentException at invalid time zone textual representation instead of defaulting to GMT #655

Merged
merged 4 commits into from
Sep 29, 2021

Conversation

brenuart
Copy link
Collaborator

Closes #650.

* between 00 to 59. For example, "GMT+10" and "GMT+0010" mean ten
* hours and ten minutes ahead of GMT, respectively.
*
* <p>Use a blank string or {@code null} to use the default TimeZone of the system.
Copy link
Collaborator Author

@brenuart brenuart Sep 22, 2021

Choose a reason for hiding this comment

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

Joran does not allow to set a blank string nor a null value... May be we need another way here for the user to say it wants the default time zone of the system...?
User can of course not set the property and keep the default value... but what if he wants to explicitly set a value and not rely on the default?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding a special case for "DEFAULT" seems ok to me.

Copy link
Collaborator Author

@brenuart brenuart Sep 24, 2021

Choose a reason for hiding this comment

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

[DEFAULT] can now be used to refer to the system default time zone (between brackets to follow same convention as used by the pattern property of the same class).
Since Joran does not allow to set an empty string or a null value, I also removed these mentions in the documentation. These options are still supported tho and are documented in the javadoc for use by programmatic configuration if someone wants to go that road.

README.md Outdated Show resolved Hide resolved
* between 00 to 59. For example, "GMT+10" and "GMT+0010" mean ten
* hours and ten minutes ahead of GMT, respectively.
*
* <p>Use a blank string or {@code null} to use the default TimeZone of the system.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding a special case for "DEFAULT" seems ok to me.

src/main/java/net/logstash/logback/util/TimeZoneUtils.java Outdated Show resolved Hide resolved
@brenuart brenuart merged commit c3781d1 into main Sep 29, 2021
@brenuart brenuart deleted the gh650-timezone branch September 29, 2021 10:37
@philsttr philsttr added this to the 7.0 milestone Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FormattedTimestampJsonProvider#setTimeZone defaults to GMT if the zoneId is not a valid zone id
2 participants