Skip to content

server.servlet.session.timeout not in effect when using Jetty starter without jakarta.annotation and javax.annotation #23716

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

Closed
daliborfilus opened this issue Oct 15, 2020 · 3 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@daliborfilus
Copy link

Example project:
https://github.com/daliborfilus/spring-boot-redis-ttl-issue

I create this issue as per request in #15757 .

The server.servlet.session.timeout does not have any effect in my application and producing this minimal example revealed the core issue. I am using Jetty starter and have tomcat starter excluded. Because of some now-strange reasons, I've excluded more dependencies than I should've.

I nailed down the conditions to this (snippet from build.gradle):

Tomcat version (To use tomcat, comment 1) and comment out 3)):
- a) session TTL works in spring boot default mode, i.e. 2) is commented and 4) is commented
- b) session TTL works if 2) is uncommented and 4) is uncommented (replace jakarta.annotation with javax.annotation)
- c) app won't boot if 2) is uncommented and 4) is commented - you need to have at least jakarta.annotation OR javax.annotation present for it to work

Jetty version (To use jetty, uncomment 1) and uncomment 3):
- a) session TTL works if 2) is uncommented and 4) is uncommented (replace jakarta.annotation with javax.annotation)
- b) session TTL works if 2) is commented and 4) is uncommented ("add" javax.annotation dependency with excluding just tomcat)
- b) session TTL doesn't work if 2) is uncommented and 4) is commented (that means no jakarta.annotation nor javax.annotation present, but the app still boots and no errors are visible)

Basically: instead of the app crashing because of missing dependencies, it still works, but Redis TTL doesn't. I don't know what else doesn't work in this situation...

I know that this is now my fault for excluding those dependencies, but I think it's strange the app works without them and this silent issue arises. But only when using Jetty starter. Jetty itself is not dependent on any of the stuff, but Spring autoconfiguration doesn't see them and so it doesn't use them... yet still everything else works.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 15, 2020
@wilkinsona wilkinsona self-assigned this Oct 16, 2020
@wilkinsona
Copy link
Member

Thanks for the sample. I understand the behaviour now.

As you have observed, Tomcat requires javax.annotation.Resource to be on the classpath and the app fails to start without it. Jetty, on the other hand, does not require any javax.annotation.* classes and is able to start without them. This then leads to the session timeout configuration being ignored due to the way that the JVM handles annotations that aren't on the classpath.

There's a method on SessionProperties that's annotated with javax.annotation.PostConstruct:

@PostConstruct
public void checkSessionTimeout() {
if (this.timeout == null && this.serverProperties != null) {
this.timeout = this.serverProperties.getServlet().getSession().getTimeout();
}
}

When @PostConstruct isn't on the classpath, the JVM silently drops it when loading the class. Spring Framework's code that looks for @PostConstruct then can't find it so it doesn't know that the methods need to be called once SessionProperties has been constructed. Getting into this situation also requires using Java 11 as Java 8 bundles the javax.annotation.* classes in the JRE.

In general terms, this is standard JVM behaviour and I don't think we should do anything to change it so that, for example, and app would always fail to start without the javax.annotation.* classes. That said, the change in the session timeout behaviour is unfortunate and I don't think we should require the javax.annotation.* classes to be present for it to work.

One way to fix this would be to switch to implementing InitializingBean rather than using @PostConstruct. Alternatively, we could choose which timeout to return in the getter and avoid the need for @PostConstruct or InitializingBean entirely. We may want to review the other places where we use @PostConstruct as well.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 16, 2020
@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Oct 16, 2020
@philwebb philwebb added this to the 2.2.x milestone Oct 16, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Oct 16, 2020

We may want to review the other places where we use @PostConstruct as well.

#23723 has been opened for this.

@daliborfilus
Copy link
Author

That is ... very interesting. One more thing I found was also broken by this was HAL response/ request? converters.

GET /api/... Accept: application/json
gave me response with { ..., "links": [{"rel": "self", "href": "http://..."}]}

but
GET /api/... Accept: application/hal+json
gave me correct HAL response with { ..., "_links": {"self": { "href": "..."}}}

I thought this was a change in Spring / HAL libraries - so it returns HAL format only for Accept: application/hal+json,
but now I found out that if I don't exclude jakarta.annotation and javax.annotation, I always HAL formatted response and I can even remove @EnableHypermediaSupport(type = [EnableHypermediaSupport.HypermediaType.HAL]) and it still works correctly.
(I use spring-boot-starter-hateoas.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants