Skip to content

Commit

Permalink
use static Loggers for CasExpirationPolicies
Browse files Browse the repository at this point in the history
Transient fields always default to null on object deserialization. This leads
to NPEs when trying to use the Logger object on a deserialized
CasExpirationPolicy, such as when a TGT containing a CasExpirationPolicy is
loaded from JPA.
  • Loading branch information
frett committed Dec 1, 2015
1 parent 6f611ee commit fb7e6ed
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ public abstract class AbstractCasExpirationPolicy implements ExpirationPolicy {

private static final long serialVersionUID = 8042104336580063690L;

/** The Logger instance shared by all children of this class. */
protected final transient Logger logger = LoggerFactory.getLogger(this.getClass());
/**
* The Logger instance for this class. Using a transient instance field for the Logger doesn't work, on object
* deserialization the field is null.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(AbstractCasExpirationPolicy.class);

/**
* Gets the http request based on the
Expand All @@ -36,7 +39,7 @@ protected final HttpServletRequest getRequest() {
return attrs.getRequest();
}
} catch (final Exception e) {
logger.trace("Unable to obtain the http request", e);
LOGGER.trace("Unable to obtain the http request", e);
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.jasig.cas.ticket.support;

import org.jasig.cas.ticket.TicketState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.util.Assert;
Expand All @@ -20,6 +22,12 @@ public final class MultiTimeUseOrTimeoutExpirationPolicy extends AbstractCasExpi
/** Serialization support. */
private static final long serialVersionUID = -5704993954986738308L;

/**
* The Logger instance for this class. Using a transient instance field for the Logger doesn't work, on object
* deserialization the field is null.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(MultiTimeUseOrTimeoutExpirationPolicy.class);

/** The time to kill in milliseconds. */
@Value("#{${st.timeToKillInSeconds:10}*1000}")
private final long timeToKillInMilliSeconds;
Expand Down Expand Up @@ -67,12 +75,12 @@ public MultiTimeUseOrTimeoutExpirationPolicy(final int numberOfUses, final long
@Override
public boolean isExpired(final TicketState ticketState) {
if (ticketState == null) {
logger.debug("Ticket state is null for {}", this.getClass().getSimpleName());
LOGGER.debug("Ticket state is null for {}", this.getClass().getSimpleName());
return true;
}
final long countUses = ticketState.getCountOfUses();
if (countUses >= this.numberOfUses) {
logger.debug("Ticket usage count {} is greater than or equal to {}", countUses, this.numberOfUses);
LOGGER.debug("Ticket usage count {} is greater than or equal to {}", countUses, this.numberOfUses);
return true;
}

Expand All @@ -81,7 +89,7 @@ public boolean isExpired(final TicketState ticketState) {
final long difference = systemTime - lastTimeUsed;

if (difference >= this.timeToKillInMilliSeconds) {
logger.debug("Ticket has expired because the difference between current time [{}] "
LOGGER.debug("Ticket has expired because the difference between current time [{}] "
+ "and ticket time [{}] is greater than or equal to [{}]", systemTime, lastTimeUsed,
this.timeToKillInMilliSeconds);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import org.jasig.cas.authentication.RememberMeCredential;
import org.jasig.cas.ticket.ExpirationPolicy;
import org.jasig.cas.ticket.TicketState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;
Expand All @@ -23,6 +25,12 @@ public final class RememberMeDelegatingExpirationPolicy extends AbstractCasExpir
/** Serialization support. */
private static final long serialVersionUID = -2735975347698196127L;

/**
* The Logger instance for this class. Using a transient instance field for the Logger doesn't work, on object
* deserialization the field is null.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(RememberMeDelegatingExpirationPolicy.class);

@Nullable
@Autowired(required=false)
@Qualifier("rememberMeExpirationPolicy")
Expand Down Expand Up @@ -52,7 +60,7 @@ public boolean isExpired(final TicketState ticketState) {

return this.rememberMeExpirationPolicy.isExpired(ticketState);
}
logger.warn("No expiration policy settings are defined");
LOGGER.warn("No expiration policy settings are defined");
return false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.jasig.cas.ticket.support;

import org.jasig.cas.ticket.TicketState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

Expand All @@ -18,6 +20,12 @@ public final class ThrottledUseAndTimeoutExpirationPolicy extends AbstractCasExp
/** Serialization support. */
private static final long serialVersionUID = 205979491183779408L;

/**
* The Logger instance for this class. Using a transient instance field for the Logger doesn't work, on object
* deserialization the field is null.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(ThrottledUseAndTimeoutExpirationPolicy.class);

/** The time to kill in milliseconds. */
@Value("#{${tgt.maxTimeToLiveInSeconds:28800}*1000}")
private long timeToKillInMilliSeconds;
Expand Down Expand Up @@ -47,18 +55,18 @@ public boolean isExpired(final TicketState ticketState) {

if (ticketState.getCountOfUses() == 0
&& (currentTimeInMillis - lastTimeTicketWasUsed < this.timeToKillInMilliSeconds)) {
logger.debug("Ticket is not expired due to a count of zero and the time being less "
LOGGER.debug("Ticket is not expired due to a count of zero and the time being less "
+ "than the timeToKillInMilliseconds");
return false;
}

if ((currentTimeInMillis - lastTimeTicketWasUsed >= this.timeToKillInMilliSeconds)) {
logger.debug("Ticket is expired due to the time being greater than the timeToKillInMilliseconds");
LOGGER.debug("Ticket is expired due to the time being greater than the timeToKillInMilliseconds");
return true;
}

if ((currentTimeInMillis - lastTimeTicketWasUsed <= this.timeInBetweenUsesInMilliSeconds)) {
logger.warn("Ticket is expired due to the time being less than the waiting period.");
LOGGER.warn("Ticket is expired due to the time being less than the waiting period.");
return true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.jasig.cas.ticket.support;

import org.jasig.cas.ticket.TicketState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
Expand All @@ -21,6 +23,12 @@ public final class TicketGrantingTicketExpirationPolicy extends AbstractCasExpir
/** Serialization support. */
private static final long serialVersionUID = 7670537200691354820L;

/**
* The Logger instance for this class. Using a transient instance field for the Logger doesn't work, on object
* deserialization the field is null.
*/
private static final Logger LOGGER = LoggerFactory.getLogger(TicketGrantingTicketExpirationPolicy.class);

/** Maximum time this ticket is valid. */
@Value("#{${tgt.maxTimeToLiveInSeconds:28800}*1000}")
private long maxTimeToLiveInMilliSeconds;
Expand Down Expand Up @@ -56,13 +64,13 @@ public boolean isExpired(final TicketState ticketState) {

// Ticket has been used, check maxTimeToLive (hard window)
if ((currentSystemTimeInMillis - ticketState.getCreationTime() >= maxTimeToLiveInMilliSeconds)) {
logger.debug("Ticket is expired because the time since creation is greater than maxTimeToLiveInMilliSeconds");
LOGGER.debug("Ticket is expired because the time since creation is greater than maxTimeToLiveInMilliSeconds");
return true;
}

// Ticket is within hard window, check timeToKill (sliding window)
if ((currentSystemTimeInMillis - ticketState.getLastTimeUsed() >= timeToKillInMilliSeconds)) {
logger.debug("Ticket is expired because the time since last use is greater than timeToKillInMilliseconds");
LOGGER.debug("Ticket is expired because the time since last use is greater than timeToKillInMilliseconds");
return true;
}

Expand Down
Empty file modified gradlew
100644 → 100755
Empty file.
Empty file modified gradlew.bat
100644 → 100755
Empty file.

0 comments on commit fb7e6ed

Please sign in to comment.