Skip to content

Commit 32462d5

Browse files
petersomogyipetersomogyi
committed
HBASE-22340 Corrupt KeyValue is silently ignored (#207)
1 parent b06ad82 commit 32462d5

File tree

2 files changed

+118
-83
lines changed

2 files changed

+118
-83
lines changed

‎hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java‎

Lines changed: 107 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import org.apache.hadoop.io.IOUtils;
3636
import org.apache.hadoop.io.WritableUtils;
3737
import org.apache.yetus.audience.InterfaceAudience;
38+
import org.slf4j.Logger;
39+
import org.slf4j.LoggerFactory;
3840

3941
import org.apache.hbase.thirdparty.com.google.common.base.Function;
4042
import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
@@ -46,6 +48,8 @@
4648
@InterfaceAudience.Private
4749
public class KeyValueUtil {
4850

51+
private static final Logger LOG = LoggerFactory.getLogger(KeyValueUtil.class);
52+
4953
/**************** length *********************/
5054

5155
public static int length(short rlen, byte flen, int qlen, int vlen, int tlen, boolean withTags) {
@@ -510,97 +514,124 @@ public static long write(final KeyValue kv, final DataOutput out) throws IOExcep
510514

511515
static String bytesToHex(byte[] buf, int offset, int length) {
512516
String bufferContents = buf != null ? Bytes.toStringBinary(buf, offset, length) : "<null>";
513-
return ", KeyValueBytesHex=" + bufferContents + ", offset=" + offset
514-
+ ", length=" + length;
517+
return ", KeyValueBytesHex=" + bufferContents + ", offset=" + offset + ", length=" + length;
515518
}
516519

517520
static void checkKeyValueBytes(byte[] buf, int offset, int length, boolean withTags) {
518521
if (buf == null) {
519-
throw new IllegalArgumentException("Invalid to have null " +
520-
"byte array in KeyValue.");
522+
String msg = "Invalid to have null byte array in KeyValue.";
523+
LOG.warn(msg);
524+
throw new IllegalArgumentException(msg);
521525
}
522526

523527
int pos = offset, endOffset = offset + length;
524528
// check the key
525529
if (pos + Bytes.SIZEOF_INT > endOffset) {
526-
throw new IllegalArgumentException(
527-
"Overflow when reading key length at position=" + pos + bytesToHex(buf, offset, length));
530+
String msg =
531+
"Overflow when reading key length at position=" + pos + bytesToHex(buf, offset, length);
532+
LOG.warn(msg);
533+
throw new IllegalArgumentException(msg);
528534
}
529535
int keyLen = Bytes.toInt(buf, pos, Bytes.SIZEOF_INT);
530536
pos += Bytes.SIZEOF_INT;
531537
if (keyLen <= 0 || pos + keyLen > endOffset) {
532-
throw new IllegalArgumentException(
533-
"Invalid key length in KeyValue. keyLength=" + keyLen + bytesToHex(buf, offset, length));
538+
String msg =
539+
"Invalid key length in KeyValue. keyLength=" + keyLen + bytesToHex(buf, offset, length);
540+
LOG.warn(msg);
541+
throw new IllegalArgumentException(msg);
534542
}
535543
// check the value
536544
if (pos + Bytes.SIZEOF_INT > endOffset) {
537-
throw new IllegalArgumentException("Overflow when reading value length at position=" + pos
538-
+ bytesToHex(buf, offset, length));
545+
String msg =
546+
"Overflow when reading value length at position=" + pos + bytesToHex(buf, offset, length);
547+
LOG.warn(msg);
548+
throw new IllegalArgumentException(msg);
539549
}
540550
int valLen = Bytes.toInt(buf, pos, Bytes.SIZEOF_INT);
541551
pos += Bytes.SIZEOF_INT;
542552
if (valLen < 0 || pos + valLen > endOffset) {
543-
throw new IllegalArgumentException("Invalid value length in KeyValue, valueLength=" + valLen
544-
+ bytesToHex(buf, offset, length));
553+
String msg = "Invalid value length in KeyValue, valueLength=" + valLen +
554+
bytesToHex(buf, offset, length);
555+
LOG.warn(msg);
556+
throw new IllegalArgumentException(msg);
545557
}
546558
// check the row
547559
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
548-
throw new IllegalArgumentException(
549-
"Overflow when reading row length at position=" + pos + bytesToHex(buf, offset, length));
560+
String msg =
561+
"Overflow when reading row length at position=" + pos + bytesToHex(buf, offset, length);
562+
LOG.warn(msg);
563+
throw new IllegalArgumentException(msg);
550564
}
551565
short rowLen = Bytes.toShort(buf, pos, Bytes.SIZEOF_SHORT);
552566
pos += Bytes.SIZEOF_SHORT;
553567
if (rowLen < 0 || pos + rowLen > endOffset) {
554-
throw new IllegalArgumentException(
555-
"Invalid row length in KeyValue, rowLength=" + rowLen + bytesToHex(buf, offset, length));
568+
String msg =
569+
"Invalid row length in KeyValue, rowLength=" + rowLen + bytesToHex(buf, offset, length);
570+
LOG.warn(msg);
571+
throw new IllegalArgumentException(msg);
556572
}
557573
pos += rowLen;
558574
// check the family
559575
if (pos + Bytes.SIZEOF_BYTE > endOffset) {
560-
throw new IllegalArgumentException("Overflow when reading family length at position=" + pos
561-
+ bytesToHex(buf, offset, length));
576+
String msg = "Overflow when reading family length at position=" + pos +
577+
bytesToHex(buf, offset, length);
578+
LOG.warn(msg);
579+
throw new IllegalArgumentException(msg);
562580
}
563581
int familyLen = buf[pos];
564582
pos += Bytes.SIZEOF_BYTE;
565583
if (familyLen < 0 || pos + familyLen > endOffset) {
566-
throw new IllegalArgumentException("Invalid family length in KeyValue, familyLength="
567-
+ familyLen + bytesToHex(buf, offset, length));
584+
String msg = "Invalid family length in KeyValue, familyLength=" + familyLen +
585+
bytesToHex(buf, offset, length);
586+
LOG.warn(msg);
587+
throw new IllegalArgumentException(msg);
568588
}
569589
pos += familyLen;
570590
// check the qualifier
571591
int qualifierLen = keyLen - Bytes.SIZEOF_SHORT - rowLen - Bytes.SIZEOF_BYTE - familyLen
572592
- Bytes.SIZEOF_LONG - Bytes.SIZEOF_BYTE;
573593
if (qualifierLen < 0 || pos + qualifierLen > endOffset) {
574-
throw new IllegalArgumentException("Invalid qualifier length in KeyValue, qualifierLen="
575-
+ qualifierLen + bytesToHex(buf, offset, length));
594+
String msg = "Invalid qualifier length in KeyValue, qualifierLen=" + qualifierLen +
595+
bytesToHex(buf, offset, length);
596+
LOG.warn(msg);
597+
throw new IllegalArgumentException(msg);
576598
}
577599
pos += qualifierLen;
578600
// check the timestamp
579601
if (pos + Bytes.SIZEOF_LONG > endOffset) {
580-
throw new IllegalArgumentException(
581-
"Overflow when reading timestamp at position=" + pos + bytesToHex(buf, offset, length));
602+
String msg =
603+
"Overflow when reading timestamp at position=" + pos + bytesToHex(buf, offset, length);
604+
LOG.warn(msg);
605+
throw new IllegalArgumentException(msg);
582606
}
583607
long timestamp = Bytes.toLong(buf, pos, Bytes.SIZEOF_LONG);
584608
if (timestamp < 0) {
585-
throw new IllegalArgumentException(
586-
"Timestamp cannot be negative, ts=" + timestamp + bytesToHex(buf, offset, length));
609+
String msg =
610+
"Timestamp cannot be negative, ts=" + timestamp + bytesToHex(buf, offset, length);
611+
LOG.warn(msg);
612+
throw new IllegalArgumentException(msg);
587613
}
588614
pos += Bytes.SIZEOF_LONG;
589615
// check the type
590616
if (pos + Bytes.SIZEOF_BYTE > endOffset) {
591-
throw new IllegalArgumentException(
592-
"Overflow when reading type at position=" + pos + bytesToHex(buf, offset, length));
617+
String msg =
618+
"Overflow when reading type at position=" + pos + bytesToHex(buf, offset, length);
619+
LOG.warn(msg);
620+
throw new IllegalArgumentException(msg);
593621
}
594622
byte type = buf[pos];
595623
if (!Type.isValidType(type)) {
596-
throw new IllegalArgumentException(
597-
"Invalid type in KeyValue, type=" + type + bytesToHex(buf, offset, length));
624+
String msg = "Invalid type in KeyValue, type=" + type + bytesToHex(buf, offset, length);
625+
LOG.warn(msg);
626+
throw new IllegalArgumentException(msg);
598627
}
599628
pos += Bytes.SIZEOF_BYTE;
600629
// check the value
601630
if (pos + valLen > endOffset) {
602-
throw new IllegalArgumentException(
603-
"Overflow when reading value part at position=" + pos + bytesToHex(buf, offset, length));
631+
String msg =
632+
"Overflow when reading value part at position=" + pos + bytesToHex(buf, offset, length);
633+
LOG.warn(msg);
634+
throw new IllegalArgumentException(msg);
604635
}
605636
pos += valLen;
606637
// check the tags
@@ -609,39 +640,55 @@ static void checkKeyValueBytes(byte[] buf, int offset, int length, boolean withT
609640
// withTags is true but no tag in the cell.
610641
return;
611642
}
612-
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
613-
throw new IllegalArgumentException("Overflow when reading tags length at position=" + pos
614-
+ bytesToHex(buf, offset, length));
615-
}
616-
short tagsLen = Bytes.toShort(buf, pos);
617-
pos += Bytes.SIZEOF_SHORT;
618-
if (tagsLen < 0 || pos + tagsLen > endOffset) {
619-
throw new IllegalArgumentException("Invalid tags length in KeyValue at position="
620-
+ (pos - Bytes.SIZEOF_SHORT) + bytesToHex(buf, offset, length));
621-
}
622-
int tagsEndOffset = pos + tagsLen;
623-
for (; pos < tagsEndOffset;) {
624-
if (pos + Tag.TAG_LENGTH_SIZE > endOffset) {
625-
throw new IllegalArgumentException("Overflow when reading tag length at position=" + pos
626-
+ bytesToHex(buf, offset, length));
627-
}
628-
short tagLen = Bytes.toShort(buf, pos);
629-
pos += Tag.TAG_LENGTH_SIZE;
630-
// tagLen contains one byte tag type, so must be not less than 1.
631-
if (tagLen < 1 || pos + tagLen > endOffset) {
632-
throw new IllegalArgumentException(
633-
"Invalid tag length at position=" + (pos - Tag.TAG_LENGTH_SIZE) + ", tagLength="
634-
+ tagLen + bytesToHex(buf, offset, length));
635-
}
636-
pos += tagLen;
637-
}
643+
pos = checkKeyValueTagBytes(buf, offset, length, pos, endOffset);
638644
}
639645
if (pos != endOffset) {
640-
throw new IllegalArgumentException("Some redundant bytes in KeyValue's buffer, startOffset="
641-
+ pos + ", endOffset=" + endOffset + bytesToHex(buf, offset, length));
646+
String msg = "Some redundant bytes in KeyValue's buffer, startOffset=" + pos + ", endOffset="
647+
+ endOffset + bytesToHex(buf, offset, length);
648+
LOG.warn(msg);
649+
throw new IllegalArgumentException(msg);
642650
}
643651
}
644652

653+
private static int checkKeyValueTagBytes(byte[] buf, int offset, int length, int pos,
654+
int endOffset) {
655+
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
656+
String msg = "Overflow when reading tags length at position=" + pos +
657+
bytesToHex(buf, offset, length);
658+
LOG.warn(msg);
659+
throw new IllegalArgumentException(msg);
660+
}
661+
short tagsLen = Bytes.toShort(buf, pos);
662+
pos += Bytes.SIZEOF_SHORT;
663+
if (tagsLen < 0 || pos + tagsLen > endOffset) {
664+
String msg = "Invalid tags length in KeyValue at position=" + (pos - Bytes.SIZEOF_SHORT)
665+
+ bytesToHex(buf, offset, length);
666+
LOG.warn(msg);
667+
throw new IllegalArgumentException(msg);
668+
}
669+
int tagsEndOffset = pos + tagsLen;
670+
for (; pos < tagsEndOffset;) {
671+
if (pos + Tag.TAG_LENGTH_SIZE > endOffset) {
672+
String msg = "Overflow when reading tag length at position=" + pos +
673+
bytesToHex(buf, offset, length);
674+
LOG.warn(msg);
675+
throw new IllegalArgumentException(msg);
676+
}
677+
short tagLen = Bytes.toShort(buf, pos);
678+
pos += Tag.TAG_LENGTH_SIZE;
679+
// tagLen contains one byte tag type, so must be not less than 1.
680+
if (tagLen < 1 || pos + tagLen > endOffset) {
681+
String msg =
682+
"Invalid tag length at position=" + (pos - Tag.TAG_LENGTH_SIZE) + ", tagLength="
683+
+ tagLen + bytesToHex(buf, offset, length);
684+
LOG.warn(msg);
685+
throw new IllegalArgumentException(msg);
686+
}
687+
pos += tagLen;
688+
}
689+
return pos;
690+
}
691+
645692
/**
646693
* Create a KeyValue reading from the raw InputStream. Named
647694
* <code>createKeyValueFromInputStream</code> so doesn't clash with {@link #create(DataInput)}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/ProtobufLogReader.java‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,7 @@ protected boolean readNext(Entry entry) throws IOException {
333333
// OriginalPosition might be < 0 on local fs; if so, it is useless to us.
334334
long originalPosition = this.inputStream.getPos();
335335
if (trailerPresent && originalPosition > 0 && originalPosition == this.walEditsStopOffset) {
336-
if (LOG.isTraceEnabled()) {
337-
LOG.trace("Reached end of expected edits area at offset " + originalPosition);
338-
}
336+
LOG.trace("Reached end of expected edits area at offset {}", originalPosition);
339337
return false;
340338
}
341339
WALKey.Builder builder = WALKey.newBuilder();
@@ -373,10 +371,8 @@ protected boolean readNext(Entry entry) throws IOException {
373371
WALKey walKey = builder.build();
374372
entry.getKey().readFieldsFromPb(walKey, this.byteStringUncompressor);
375373
if (!walKey.hasFollowingKvCount() || 0 == walKey.getFollowingKvCount()) {
376-
if (LOG.isTraceEnabled()) {
377-
LOG.trace("WALKey has no KVs that follow it; trying the next one. current offset=" +
378-
this.inputStream.getPos());
379-
}
374+
LOG.trace("WALKey has no KVs that follow it; trying the next one. current offset={}",
375+
this.inputStream.getPos());
380376
seekOnFs(originalPosition);
381377
return false;
382378
}
@@ -393,9 +389,7 @@ protected boolean readNext(Entry entry) throws IOException {
393389
try {
394390
posAfterStr = this.inputStream.getPos() + "";
395391
} catch (Throwable t) {
396-
if (LOG.isTraceEnabled()) {
397-
LOG.trace("Error getting pos for error message - ignoring", t);
398-
}
392+
LOG.trace("Error getting pos for error message - ignoring", t);
399393
}
400394
String message = " while reading " + expectedCells + " WAL KVs; started reading at "
401395
+ posBefore + " and read up to " + posAfterStr;
@@ -412,27 +406,21 @@ protected boolean readNext(Entry entry) throws IOException {
412406
} catch (EOFException eof) {
413407
// If originalPosition is < 0, it is rubbish and we cannot use it (probably local fs)
414408
if (originalPosition < 0) {
415-
if (LOG.isTraceEnabled()) {
416-
LOG.trace("Encountered a malformed edit, but can't seek back to last good position "
417-
+ "because originalPosition is negative. last offset="
418-
+ this.inputStream.getPos(), eof);
419-
}
409+
LOG.warn("Encountered a malformed edit, but can't seek back to last good position "
410+
+ "because originalPosition is negative. last offset={}",
411+
this.inputStream.getPos(), eof);
420412
throw eof;
421413
}
422414
// If stuck at the same place and we got and exception, lets go back at the beginning.
423415
if (inputStream.getPos() == originalPosition && resetPosition) {
424-
if (LOG.isTraceEnabled()) {
425-
LOG.trace("Encountered a malformed edit, seeking to the beginning of the WAL since "
426-
+ "current position and original position match at " + originalPosition);
427-
}
416+
LOG.warn("Encountered a malformed edit, seeking to the beginning of the WAL since "
417+
+ "current position and original position match at {}", originalPosition);
428418
seekOnFs(0);
429419
} else {
430420
// Else restore our position to original location in hope that next time through we will
431421
// read successfully.
432-
if (LOG.isTraceEnabled()) {
433-
LOG.trace("Encountered a malformed edit, seeking back to last good position in file, "
434-
+ "from " + inputStream.getPos()+" to " + originalPosition, eof);
435-
}
422+
LOG.warn("Encountered a malformed edit, seeking back to last good position in file, "
423+
+ "from {} to {}", inputStream.getPos(), originalPosition, eof);
436424
seekOnFs(originalPosition);
437425
}
438426
return false;

0 commit comments

Comments
 (0)