Fix MysqlDataTruncation exception when user open nonexistent page with long URL.

Abbreviate user's data (page, user agent and referrer) before saving to the database.

Author: AleksSPb

src/main/java/ru/mystamps/web/entity/SuspiciousActivity.java

@@ -36,9 +36,11 @@
 @Setter
 public class SuspiciousActivity {
 
-	public static final int PAGE_URL_LENGTH = 100;
-	public static final int IP_LENGTH       = 15;
-	public static final int METHOD_LENGTH   = 7;
+	public static final int PAGE_URL_LENGTH     = 100;
+	public static final int METHOD_LENGTH       = 7;
+	public static final int IP_LENGTH           = 15;
+	public static final int REFERER_PAGE_LENGTH = 255;
+	public static final int USER_AGENT_LENGTH   = 255;
 
 	@Id
 	@GeneratedValue
@@ -62,10 +64,10 @@ public class SuspiciousActivity {
 	@Column(length = IP_LENGTH, nullable = false)
 	private String ip;
 
-	@Column(name = "referer_page", nullable = false)
+	@Column(name = "referer_page", length = REFERER_PAGE_LENGTH, nullable = false)
 	private String refererPage;
 
-	@Column(name = "user_agent", nullable = false)
+	@Column(name = "user_agent", length = USER_AGENT_LENGTH, nullable = false)
 	private String userAgent;
 
 }

src/main/java/ru/mystamps/web/service/SiteServiceImpl.java

@@ -22,6 +22,8 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -35,6 +37,8 @@
 @RequiredArgsConstructor
 public class SiteServiceImpl implements SiteService {
 
+	private static final Logger LOG = LoggerFactory.getLogger(SiteServiceImpl.class);
+	
 	// see initiate-suspicious_activities_types-table changeset
 	// in src/main/resources/liquibase/initial-state.xml
 	private static final String PAGE_NOT_FOUND = "PageNotFound";
@@ -94,16 +98,48 @@ private void logEvent(
 		activity.setType(activityType);
 
 		activity.setOccurredAt(date == null ? new Date() : date);
-		activity.setPage(page);
+		activity.setPage(abbreviatePage(page));
 		activity.setMethod(method);
 
 		activity.setUser(user);
 
 		activity.setIp(StringUtils.defaultString(ip));
-		activity.setRefererPage(StringUtils.defaultString(referer));
-		activity.setUserAgent(StringUtils.defaultString(agent));
+		activity.setRefererPage(abbreviateRefererPage(StringUtils.defaultString(referer)));
+		activity.setUserAgent(abbreviateUserAgent(StringUtils.defaultString(agent)));
 
 		suspiciousActivities.add(activity);
 	}
 
+	private static String abbreviatePage(String page) {
+		return abbreviateIfLengthGreaterThan(page, SuspiciousActivity.PAGE_URL_LENGTH, "page");
+	}
+	
+	private static String abbreviateRefererPage(String referer) {
+		// CheckStyle: ignore LineLength for next 1 lines
+		return abbreviateIfLengthGreaterThan(referer, SuspiciousActivity.REFERER_PAGE_LENGTH, "referer_page");
+	}
+	
+	private static String abbreviateUserAgent(String agent) {
+		// CheckStyle: ignore LineLength for next 1 lines
+		return abbreviateIfLengthGreaterThan(agent, SuspiciousActivity.USER_AGENT_LENGTH, "user_agent");
+	}
+	
+	// CheckStyle: ignore LineLength for next 1 lines
+	private static String abbreviateIfLengthGreaterThan(String text, int maxLength, String fieldName) {
+		if (text == null || text.length() <= maxLength) {
+			return text;
+		}
+		
+		// TODO(security): fix possible log injection
+		LOG.warn(
+				"Length of value for '{}' field ({}) exceeds max field size ({}): '{}'",
+				fieldName,
+				text.length(),
+				maxLength,
+				text
+		);
+		
+		return StringUtils.abbreviate(text, maxLength);
+	}
+	
 }

src/test/groovy/ru/mystamps/web/service/SiteServiceImplTest.groovy

@@ -89,7 +89,21 @@ class SiteServiceImplTest extends Specification {
 				return true
 			})
 	}
-
+	
+	def "logAboutAbsentPage() should pass abbreviated page when it's too long"() {
+		given:
+			String longPageUrl = '/long/url/' + ('x' * SuspiciousActivity.PAGE_URL_LENGTH)
+		and:
+			String expectedPageUrl = longPageUrl.take(SuspiciousActivity.PAGE_URL_LENGTH - 3) + '...'
+		when:
+			service.logAboutAbsentPage(longPageUrl, TEST_METHOD, null, null, null, null)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.page == expectedPageUrl
+				return true
+			})
+	}
+	
 	@Unroll
 	def "logAboutAbsentPage() should pass method to dao"(String expectedMethod) {
 		when:
@@ -155,6 +169,19 @@ class SiteServiceImplTest extends Specification {
 				return true
 			})
 	}
+	def "logAboutAbsentPage() should pass abbreviated referer when it's too long"() {
+		given:
+			String longRefererUrl = '/long/url/' + ('x' * SuspiciousActivity.REFERER_PAGE_LENGTH)
+		and:
+			String expectedRefererUrl = longRefererUrl.take(SuspiciousActivity.REFERER_PAGE_LENGTH - 3) + '...'
+		when:
+			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, longRefererUrl, null)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.refererPage == expectedRefererUrl
+				return true
+			})
+	}
 
 	def "logAboutAbsentPage() should pass empty string to dao for unknown referer"() {
 		when:
@@ -175,6 +202,19 @@ class SiteServiceImplTest extends Specification {
 				return true
 			})
 	}
+	def "logAboutAbsentPage() should pass abbreviated user agent when it's too long"() {
+		given:
+			String longUserAgent = 'Mozilla/5.0 (Windows NT 6.1) AppleWebKit/' + ('x' * SuspiciousActivity.USER_AGENT_LENGTH)
+		and:
+			String expectedUserAgent = longUserAgent.take(SuspiciousActivity.USER_AGENT_LENGTH - 3) + '...'
+		when:
+			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, null, longUserAgent)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.userAgent == expectedUserAgent
+				return true
+			})
+	}
 
 	def "logAboutAbsentPage() should pass empty string to dao for unknown user agent"() {
 		when:
@@ -247,6 +287,19 @@ class SiteServiceImplTest extends Specification {
 				return true
 			})
 	}
+	def "logAboutFailedAuthentication() should pass abbreviated page when it's too long"() {
+		given:
+			String longPageUrl = '/long/url/' + ('x' * SuspiciousActivity.PAGE_URL_LENGTH)
+		and:
+			String expectedPageUrl = longPageUrl.take(SuspiciousActivity.PAGE_URL_LENGTH - 3) + '...'
+		when:
+			service.logAboutFailedAuthentication(longPageUrl, TEST_METHOD, null, null, null, null, null)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.page == expectedPageUrl
+				return true
+			})
+	}
 
 	def "logAboutFailedAuthentication() should pass null to dao for unknown user"() {
 		when:
@@ -310,6 +363,20 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
+	def "logAboutFailedAuthentication() should pass abbreviated referer when it's too long"() {
+		given:
+			String longRefererUrl = '/long/url/' + ('x' * SuspiciousActivity.REFERER_PAGE_LENGTH)
+		and:
+			String expectedRefererUrl = longRefererUrl.take(SuspiciousActivity.REFERER_PAGE_LENGTH - 3) + '...'
+		when:
+			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, longRefererUrl, null, null)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.refererPage == expectedRefererUrl
+				return true
+			})
+	}
+	
 	def "logAboutFailedAuthentication() should pass user agent to dao"() {
 		when:
 			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, null, TEST_USER_AGENT, null)
@@ -330,4 +397,18 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
+	def "logAboutFailedAuthentication() should pass abbreviated user agent when it's too long"() {
+		given:
+			String longUserAgent = 'Mozilla/5.0 (Windows NT 6.1) AppleWebKit/' + ('x' * SuspiciousActivity.USER_AGENT_LENGTH)
+		and:
+			String expectedUserAgent = longUserAgent.take(SuspiciousActivity.USER_AGENT_LENGTH - 3) + '...'
+		when:
+			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, null, longUserAgent, null)
+		then:
+			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
+				assert activity?.userAgent == expectedUserAgent
+				return true
+			})
+	}
+	
 }