Skip to content

Fix HTTP session timestamp units #1621

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.

Sign up for GitHub

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

Open
codingkiddo wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −4
Open

Fix HTTP session timestamp units #1621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...er-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public AwsHttpSession(String id) {
}
this.id = id;
attributes = new HashMap<>();
creationTime = Instant.now().getEpochSecond();
creationTime = Instant.now().toEpochMilli();
maxInactiveInterval = SESSION_DURATION_SEC;
lastAccessedTime = creationTime;
valid = true;
Expand Down Expand Up @@ -122,11 +122,11 @@ public boolean isNew() {
}

private void touch() {
lastAccessedTime = Instant.now().getEpochSecond();
lastAccessedTime = Instant.now().toEpochMilli();

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] Switching touch() to millisecond resolution regresses isNew(), which is implemented as an equality check on the two timestamps (line 121):

public boolean isNew() {
   return lastAccessedTime == creationTime;
}

With second granularity, any getAttribute/setAttribute/removeAttribute call occurring in the same wall-clock second as construction left lastAccessedTime equal to creationTime, so isNew() stayed true for the duration of a typical sub-second Lambda invocation. With toEpochMilli(), the first attribute access almost always advances the timestamp, so isNew() starts returning false within the very same request that created the session.

That contradicts the HttpSession contract: isNew() must return true until the client has joined the session (i.e. until the client sends back the session id on a subsequent request). Since a session here is created per request and never returned by the client, it should remain new for its whole lifetime. Frameworks layered on top (for example Spring Security's session-fixation and "session created" handling) branch on isNew(), so the flip is externally observable.

Decoupling isNew() from the timestamps keeps the fix to units only:

private boolean isNew = true;

@Override
public boolean isNew() {
   return isNew;
}

Note that the existing test validSession_expectCorrectValidationOrInvalidation asserts assertFalse(sess.isNew()) after a Thread.sleep(1000), so it passes either way and does not cover this behavior change.

}

boolean isValid() {
if (lastAccessedTime - creationTime < maxInactiveInterval) {
if (lastAccessedTime - creationTime < maxInactiveInterval * 1000L) {

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[BUG] The unit conversion here is correct, but it cements a comparison that measures the wrong quantity: lastAccessedTime - creationTime is the session's total age, not its idle time. maxInactiveInterval is defined as the time the container will allow between client requests before invalidating the session, so a continuously used session is invalidated once it is older than the interval, even though it was never idle. Conversely, a session that has been idle for hours is still reported valid as long as it was created recently and never touched again.

The check should compare current time against the last access:

boolean isValid() {
   if (Instant.now().toEpochMilli() - lastAccessedTime < maxInactiveInterval  1000L) {
       return valid;
   } else {
       return false;
   }
}

While touching this line, also consider the spec rule that a zero or negative maxInactiveInterval (settable via setMaxInactiveInterval) means the session never expires. Today a negative value makes the comparison immediately false, so isValid() returns false for a brand-new session — the exact opposite of the intended "never time out" behavior.

return valid;
} else {
return false;
Expand Down
8 changes: 7 additions & 1 deletion ...ore/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpSessionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,14 @@ void new_withValidId_setsIdCorrectly() {

@Test
void new_creationTimePopulatedCorrectly() {
long beforeCreation = Instant.now().toEpochMilli();

AwsHttpSession session = new AwsHttpSession("id");
assertTrue(session.getCreationTime() > Instant.now().getEpochSecond() - 1);

long afterCreation = Instant.now().toEpochMilli();

assertTrue(session.getCreationTime() >= beforeCreation);
assertTrue(session.getCreationTime() <= afterCreation);
assertEquals(AwsHttpSession.SESSION_DURATION_SEC, session.getMaxInactiveInterval());
assertEquals(session.getLastAccessedTime(), session.getCreationTime());
}
Expand Down