Author: slebresne Date: Tue Apr 19 12:51:56 2011 New Revision: 1095070 URL: http://svn.apache.org/viewvc?rev=1095070&view=rev Log: Make scrub validate column fields patch by slebresne; reviewed by jbellis for CASSANDRA-2460
Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java Tue Apr 19 12:51:56 2011 @@ -26,7 +26,9 @@ import java.util.Collection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.util.DataOutputBuffer; import org.apache.cassandra.utils.ByteBufferUtil; @@ -237,5 +239,19 @@ public class Column implements IColumn { return !isMarkedForDelete(); } + + protected void validateName(CFMetaData metadata) throws MarshalException + { + AbstractType nameValidator = metadata.cfType == ColumnFamilyType.Super ? metadata.subcolumnComparator : metadata.comparator; + nameValidator.validate(name()); + } + + public void validateFields(CFMetaData metadata) throws MarshalException + { + validateName(metadata); + AbstractType valueValidator = metadata.getValueValidator(name()); + if (valueValidator != null) + valueValidator.validate(value()); + } } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java Tue Apr 19 12:51:56 2011 @@ -38,6 +38,7 @@ import org.apache.cassandra.config.Datab import org.apache.cassandra.db.filter.QueryPath; import org.apache.cassandra.db.marshal.AbstractCommutativeType; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.IColumnSerializer; import org.apache.cassandra.io.util.IIterableColumns; import org.apache.cassandra.utils.FBUtilities; @@ -432,4 +433,18 @@ public class ColumnFamily implements ICo size += column.serializedSize(); return size; } + + /** + * Goes over all columns and check the fields are valid (as far as we can + * tell). + * This is used to detect corruption after deserialization. + */ + public void validateColumnFields() throws MarshalException + { + CFMetaData metadata = metadata(); + for (IColumn column : getSortedColumns()) + { + column.validateFields(metadata); + } + } } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java Tue Apr 19 12:51:56 2011 @@ -27,9 +27,11 @@ import java.util.Map; import org.apache.log4j.Logger; +import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.context.CounterContext; import org.apache.cassandra.db.context.IContext.ContextRelationship; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.util.DataOutputBuffer; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; @@ -204,6 +206,15 @@ public class CounterColumn extends Colum return ColumnSerializer.COUNTER_MASK; } + @Override + public void validateFields(CFMetaData metadata) throws MarshalException + { + validateName(metadata); + // We cannot use the value validator as for other columns as the CounterColumnType validate a long, + // which is not the internal representation of counters + contextManager.validateContext(value()); + } + /** * Check if a given nodeId is found in this CounterColumn context. */ @@ -269,4 +280,5 @@ public class CounterColumn extends Colum } } } + } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java Tue Apr 19 12:51:56 2011 @@ -20,6 +20,8 @@ package org.apache.cassandra.db; import java.nio.ByteBuffer; +import org.apache.cassandra.config.CFMetaData; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.utils.ByteBufferUtil; public class DeletedColumn extends Column @@ -71,4 +73,14 @@ public class DeletedColumn extends Colum { return ColumnSerializer.DELETION_MASK; } + + @Override + public void validateFields(CFMetaData metadata) throws MarshalException + { + validateName(metadata); + if (value().remaining() != 4) + throw new MarshalException("A tombstone value should be 4 bytes long"); + if (getLocalDeletionTime() < 0) + throw new MarshalException("The local deletion time should not be negative"); + } } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java Tue Apr 19 12:51:56 2011 @@ -22,7 +22,9 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.security.MessageDigest; +import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.util.DataOutputBuffer; import org.apache.cassandra.utils.ByteBufferUtil; @@ -137,4 +139,14 @@ public class ExpiringColumn extends Colu { return ColumnSerializer.EXPIRATION_MASK; } + + @Override + public void validateFields(CFMetaData metadata) throws MarshalException + { + super.validateFields(metadata); + if (timeToLive <= 0) + throw new MarshalException("A column TTL should be > 0"); + if (localExpirationTime < 0) + throw new MarshalException("The local expiration time should not be negative"); + } } Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java Tue Apr 19 12:51:56 2011 @@ -22,7 +22,9 @@ import java.nio.ByteBuffer; import java.security.MessageDigest; import java.util.Collection; +import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.utils.FBUtilities; public interface IColumn @@ -46,6 +48,7 @@ public interface IColumn public void updateDigest(MessageDigest digest); public int getLocalDeletionTime(); // for tombstone GC, so int is sufficient granularity public String getString(AbstractType comparator); + public void validateFields(CFMetaData metadata) throws MarshalException; /** clones the column, interning column names and making copies of other underlying byte buffers * @param cfs*/ Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java Tue Apr 19 12:51:56 2011 @@ -29,7 +29,9 @@ import java.util.concurrent.ConcurrentSk import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.IColumnSerializer; import org.apache.cassandra.io.util.ColumnSortedMap; import org.apache.cassandra.io.util.DataOutputBuffer; @@ -321,6 +323,15 @@ public class SuperColumn implements ICol { throw new UnsupportedOperationException("Super columns don't have a serialization mask"); } + + public void validateFields(CFMetaData metadata) throws MarshalException + { + metadata.comparator.validate(name()); + for (IColumn column : getSubColumns()) + { + column.validateFields(metadata); + } + } } class SuperColumnSerializer implements IColumnSerializer Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java Tue Apr 19 12:51:56 2011 @@ -21,6 +21,7 @@ import java.nio.ByteBuffer; import java.security.MessageDigest; import java.util.*; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.db.DBConstants; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.NodeId; @@ -452,6 +453,13 @@ public class CounterContext implements I return cleaned; } + public void validateContext(ByteBuffer context) throws MarshalException + { + int headerLength = headerLength(context); + if (headerLength < 0 || (context.remaining() - headerLength) % STEP_LENGTH != 0) + throw new MarshalException("Invalid size for a counter context"); + } + /** * Update a MessageDigest with the content of a context. * Note that this skips the header entirely since the header information Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java?rev=1095070&r1=1095069&r2=1095070&view=diff ============================================================================== --- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java (original) +++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java Tue Apr 19 12:51:56 2011 @@ -35,6 +35,7 @@ import org.apache.cassandra.db.ColumnFam import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.IColumn; import org.apache.cassandra.db.columniterator.IColumnIterator; +import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.io.util.BufferedRandomAccessFile; import org.apache.cassandra.utils.Filter; @@ -56,6 +57,8 @@ public class SSTableIdentityIterator imp // Used by lazilyCompactedRow, so that we see the same things when deserializing the first and second time private final int expireBefore; + private final boolean validateColumns; + /** * Used to iterate through the columns of a row. * @param sstable SSTable we are reading ffrom. @@ -71,10 +74,20 @@ public class SSTableIdentityIterator imp this(sstable, file, key, dataStart, dataSize, false); } - public SSTableIdentityIterator(SSTableReader sstable, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean deserializeRowHeader) + /** + * Used to iterate through the columns of a row. + * @param sstable SSTable we are reading ffrom. + * @param file Reading using this file. + * @param key Key of this row. + * @param dataStart Data for this row starts at this pos. + * @param dataSize length of row data + * @param checkData if true, do its best to deserialize and check the coherence of row data + * @throws IOException + */ + public SSTableIdentityIterator(SSTableReader sstable, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean checkData) throws IOException { - this(sstable.metadata, file, key, dataStart, dataSize, deserializeRowHeader, sstable, false); + this(sstable.metadata, file, key, dataStart, dataSize, checkData, sstable, false); } public SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean fromRemote) @@ -84,7 +97,7 @@ public class SSTableIdentityIterator imp } // sstable may be null *if* deserializeRowHeader is false - private SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean deserializeRowHeader, SSTableReader sstable, boolean fromRemote) + private SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean checkData, SSTableReader sstable, boolean fromRemote) throws IOException { this.file = file; @@ -93,12 +106,13 @@ public class SSTableIdentityIterator imp this.dataSize = dataSize; this.expireBefore = (int)(System.currentTimeMillis() / 1000); this.fromRemote = fromRemote; + this.validateColumns = checkData; finishedAt = dataStart + dataSize; try { file.seek(this.dataStart); - if (deserializeRowHeader) + if (checkData) { try { @@ -155,12 +169,19 @@ public class SSTableIdentityIterator imp { try { - return columnFamily.getColumnSerializer().deserialize(file, null, fromRemote, expireBefore); + IColumn column = columnFamily.getColumnSerializer().deserialize(file, null, fromRemote, expireBefore); + if (validateColumns) + column.validateFields(columnFamily.metadata()); + return column; } catch (IOException e) { throw new IOError(e); } + catch (MarshalException e) + { + throw new IOError(new IOException("Error validating row " + key, e)); + } } public void remove() @@ -192,6 +213,17 @@ public class SSTableIdentityIterator imp file.seek(columnPosition - 4); // seek to before column count int ColumnFamily cf = columnFamily.cloneMeShallow(); ColumnFamily.serializer().deserializeColumns(file, cf, false, fromRemote); + if (validateColumns) + { + try + { + cf.validateColumnFields(); + } + catch (MarshalException e) + { + throw new IOException("Error validating row " + key, e); + } + } return cf; }