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

Hibernate Validator Failure When TZ Set to Non-UTC #32831

Closed
this-user opened this issue Apr 21, 2023 · 10 comments · Fixed by #43096
Closed

Hibernate Validator Failure When TZ Set to Non-UTC #32831

this-user opened this issue Apr 21, 2023 · 10 comments · Fixed by #43096
Labels
area/hibernate-validator Hibernate Validator kind/bug Something isn't working
Milestone

Comments

@this-user
Copy link

Describe the bug

Annotation-based validation via the Hibernate Validator module can fail in native mode if the time zone is set to anything but UTC. This only applies to running a native build inside a container. The container's OS time zone is set to local time, but in order to make the Quarkus application run in local time, too, setting the TZ environment variable is necessary. However, if this variable is set to anything but UTC, validation on time-based types can fail.

Specifically, I am using @PastOrPresent on a LocalDateTime field that is part of an entity class. The LocalDateTime is set to LocalDateTime.now(), so this should not fail when validation is being run. However, under the aforementioned circumstances of having set TZ to anything that has a positive offset to UTC, an exception is thrown, stating that the time must be in the past or present.

Seemingly, the explanation for this is that validation is internally still using UTC while the rest of the Quarkus application is running with the TZ that has been set. I have tried setting quarkus.hibernate-orm.jdbc.timezone to the same TZ, but this has no effect.

The issue only occurs for native builds, and does not occur for JVM-based images.

I am using the quarkus-micro-image:2.0 as the base.

The issue occurs when building for Java 17 with the Mandel builder image (v22.3) as well as when using Java 19 with GraalVM CE (v22.3).

Expected behavior

Validation should pass, because the field is valid.

Actual behavior

Validation throws an exception.

How to Reproduce?

https://github.com/this-user/quarkus-hibernate-validation-issue-tz

Steps to reproduces:

  1. Comment out quarkus.test.arg-line that sets the TZ in application.properties
  2. Run ./gradlew testNative --info
  3. Integration tests pass
  4. Remove comment from 1. and run tests again, now with non-UTC TZ
  5. Tests will fail because validation of TestEntity fails

Output of uname -a or ver

Linux 6.2.11-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 13 Apr 2023 16:59:24 +0000 x86_64 GNU/Linux

Output of java -version

17.0.6

GraalVM version (if different from Java)

22.3.1.0

Quarkus version or git rev

2.16.6 Final

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 8.0.2

Additional information

No response

@this-user this-user added the kind/bug Something isn't working label Apr 21, 2023
@quarkus-bot quarkus-bot bot added the area/hibernate-validator Hibernate Validator label Apr 21, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 21, 2023

/cc @gsmet (hibernate-validator), @yrodiere (hibernate-validator)

@this-user
Copy link
Author

Is anyone actually looking at this?

@gsmet
Copy link
Member

gsmet commented Aug 24, 2024

@marko-bekhta could you check if this is still an issue when you have some time?

We probably need to update the reproducer with quarkus update.

@marko-bekhta
Copy link
Contributor

Hey @gsmet 😃 👋🏻

Yeah, it seems that this is still an issue ... I'll try to debug it a bit more, but after a quick check, what I've noticed was that: if you access a clock provider from an injected ValidatorFactory we get an expected

clock tz: Europe/Brussels
LocalDateTime.now( from HV clock ): 2024-08-25T10:47:07.764884
LocalDateTime.now(): 2024-08-25T10:47:07.764796

but what happens inside the injected Validator instance is a bit different:

reference clock: SystemClock[GMT]
reference value: 2024-08-25T08:47:07.764978
value (LocalDateTime.now()): 2024-08-25T10:47:07.764621

@marko-bekhta
Copy link
Contributor

ok, so I think that I've found where the problem comes from. In here:

https://github.com/hibernate/hibernate-validator/blob/d2694eced589af7a279e366984e797fc026491e9/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ConstraintTree.java#L55-L60

We initialize the constraints, and in the case of date-time ones, we get a clock instance stored. At build time, it would be a GMT clock, so the clock is set to a build zone, and at runtime, it may be different... maybe we should use some custom clock like:

private static class CustomClock extends Clock {

    private ZoneId zoneId;

    @Override
    public ZoneId getZone() {
        if (zoneId == null) {
            synchronized (this) {
                if (zoneId == null) {
                    zoneId = ZoneId.systemDefault();
                }
            }
        }
        return zoneId;
    }

    @Override
    public Clock withZone(ZoneId zone) {
        throw new UnsupportedOperationException();
    }

    @Override
    public Instant instant() {
        return Instant.now();
    }
}

to let it get initialized on the first validation call?

@gsmet
Copy link
Member

gsmet commented Aug 26, 2024

Hum. That's actually a great find. Having a Clock in the native image is definitely a problem.

Not sure about how best we can fix it but... we need to be careful as to not be too slow (and your code probably needs a volatile and then to optimize the volatile reads).

@marko-bekhta
Copy link
Contributor

Another idea that I thought of... can we rely on a startup event to initialize the time zone? In other words, what if we add clock provider bean like:

public class StartupAwareClockProvider implements ClockProvider {
    private final StartupAwareClock clock = new StartupAwareClock();
    private ZoneId zoneId;

    void onStart(@Observes StartupEvent event) {
        // init the zone to match the one that is set at runtime:
        zoneId = ZoneId.systemDefault();
    }

    @Override
    public Clock getClock() {
        return clock;
    }

    private class StartupAwareClock extends Clock {

        @Override
        public ZoneId getZone() {
            return zoneId;
        }

        @Override
        public Clock withZone(ZoneId zone) {
            if (zoneId.equals(zone)) {
                return this;
            }
            return Clock.system(zone);
        }

        @Override
        public Instant instant() {
            return Instant.now();
        }
    }
}

Or, there may be something triggering validation before the event is fired?

@gsmet
Copy link
Member

gsmet commented Aug 29, 2024

This problem will only occur if we use the default Clock right?

I think you're onto something but it seems a bit brittle and I wonder if we should just reinitialize your Clock at runtime and have the zone in a static field that would be reinitialized.

See RuntimeReinitializedClassBuildItem.

That way we would rely on proven GraalVM mechanism and the value with be switched as soon as we are in runtime mode.

Does it make sense?

@marko-bekhta
Copy link
Contributor

This problem will only occur if we use the default Clock right?

I guess if a user decides to supply a custom one, but implements it as something like:

public class UserClockProvider implements ClockProvider {
    @Override
    public Clock getClock() {
        return Clock.systemDefaultZone();
    }
}

then the app will get into the same state, I guess...

but it seems a bit brittle

yeah 😃 I was just thinking if there was a way to do it safely, and I thought you'd know of one if there is 😃

Does it make sense?

yes, and I can give that a try. Maybe for the user-provided ones, we can add a warning to the documentation that users should be aware that this may happen and that they should address it on their side... 🙈

@gsmet
Copy link
Member

gsmet commented Aug 29, 2024

For the user provided ones, I would expect them to use a non-default clock and in this case, it wouldn't be a problem.

I think we can start by fixing the default case and then when someone complains with a use case, we can see how we can iterate?

But sure you can add a warning if you can write one that is clear enough for people to understand how to fix it :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-validator Hibernate Validator kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants