Skip to content

Commit 5905051

Browse files
petersomogyipetersomogyi
committed
HBASE-22340 Corrupt KeyValue is silently ignored (#207)
1 parent 813e925 commit 5905051

File tree

2 files changed

+120
-79
lines changed

2 files changed

+120
-79
lines changed

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

Lines changed: 109 additions & 56 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
/**
@@ -524,87 +528,120 @@ static String bytesToHex(byte[] buf, int offset, int length) {
524528
}
525529

526530
static void checkKeyValueBytes(byte[] buf, int offset, int length, boolean withTags) {
531+
if (buf == null) {
532+
String msg = "Invalid to have null byte array in KeyValue.";
533+
LOG.warn(msg);
534+
throw new IllegalArgumentException(msg);
535+
}
536+
527537
int pos = offset, endOffset = offset + length;
528538
// check the key
529539
if (pos + Bytes.SIZEOF_INT > endOffset) {
530-
throw new IllegalArgumentException(
531-
"Overflow when reading key length at position=" + pos + bytesToHex(buf, offset, length));
540+
String msg =
541+
"Overflow when reading key length at position=" + pos + bytesToHex(buf, offset, length);
542+
LOG.warn(msg);
543+
throw new IllegalArgumentException(msg);
532544
}
533545
int keyLen = Bytes.toInt(buf, pos, Bytes.SIZEOF_INT);
534546
pos += Bytes.SIZEOF_INT;
535547
if (keyLen <= 0 || pos + keyLen > endOffset) {
536-
throw new IllegalArgumentException(
537-
"Invalid key length in KeyValue. keyLength=" + keyLen + bytesToHex(buf, offset, length));
548+
String msg =
549+
"Invalid key length in KeyValue. keyLength=" + keyLen + bytesToHex(buf, offset, length);
550+
LOG.warn(msg);
551+
throw new IllegalArgumentException(msg);
538552
}
539553
// check the value
540554
if (pos + Bytes.SIZEOF_INT > endOffset) {
541-
throw new IllegalArgumentException("Overflow when reading value length at position=" + pos
542-
+ bytesToHex(buf, offset, length));
555+
String msg =
556+
"Overflow when reading value length at position=" + pos + bytesToHex(buf, offset, length);
557+
LOG.warn(msg);
558+
throw new IllegalArgumentException(msg);
543559
}
544560
int valLen = Bytes.toInt(buf, pos, Bytes.SIZEOF_INT);
545561
pos += Bytes.SIZEOF_INT;
546562
if (valLen < 0 || pos + valLen > endOffset) {
547-
throw new IllegalArgumentException("Invalid value length in KeyValue, valueLength=" + valLen
548-
+ bytesToHex(buf, offset, length));
563+
String msg = "Invalid value length in KeyValue, valueLength=" + valLen +
564+
bytesToHex(buf, offset, length);
565+
LOG.warn(msg);
566+
throw new IllegalArgumentException(msg);
549567
}
550568
// check the row
551569
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
552-
throw new IllegalArgumentException(
553-
"Overflow when reading row length at position=" + pos + bytesToHex(buf, offset, length));
570+
String msg =
571+
"Overflow when reading row length at position=" + pos + bytesToHex(buf, offset, length);
572+
LOG.warn(msg);
573+
throw new IllegalArgumentException(msg);
554574
}
555575
short rowLen = Bytes.toShort(buf, pos, Bytes.SIZEOF_SHORT);
556576
pos += Bytes.SIZEOF_SHORT;
557577
if (rowLen < 0 || pos + rowLen > endOffset) {
558-
throw new IllegalArgumentException(
559-
"Invalid row length in KeyValue, rowLength=" + rowLen + bytesToHex(buf, offset, length));
578+
String msg =
579+
"Invalid row length in KeyValue, rowLength=" + rowLen + bytesToHex(buf, offset, length);
580+
LOG.warn(msg);
581+
throw new IllegalArgumentException(msg);
560582
}
561583
pos += rowLen;
562584
// check the family
563585
if (pos + Bytes.SIZEOF_BYTE > endOffset) {
564-
throw new IllegalArgumentException("Overflow when reading family length at position=" + pos
565-
+ bytesToHex(buf, offset, length));
586+
String msg = "Overflow when reading family length at position=" + pos +
587+
bytesToHex(buf, offset, length);
588+
LOG.warn(msg);
589+
throw new IllegalArgumentException(msg);
566590
}
567591
int familyLen = buf[pos];
568592
pos += Bytes.SIZEOF_BYTE;
569593
if (familyLen < 0 || pos + familyLen > endOffset) {
570-
throw new IllegalArgumentException("Invalid family length in KeyValue, familyLength="
571-
+ familyLen + bytesToHex(buf, offset, length));
594+
String msg = "Invalid family length in KeyValue, familyLength=" + familyLen +
595+
bytesToHex(buf, offset, length);
596+
LOG.warn(msg);
597+
throw new IllegalArgumentException(msg);
572598
}
573599
pos += familyLen;
574600
// check the qualifier
575601
int qualifierLen = keyLen - Bytes.SIZEOF_SHORT - rowLen - Bytes.SIZEOF_BYTE - familyLen
576602
- Bytes.SIZEOF_LONG - Bytes.SIZEOF_BYTE;
577603
if (qualifierLen < 0 || pos + qualifierLen > endOffset) {
578-
throw new IllegalArgumentException("Invalid qualifier length in KeyValue, qualifierLen="
579-
+ qualifierLen + bytesToHex(buf, offset, length));
604+
String msg = "Invalid qualifier length in KeyValue, qualifierLen=" + qualifierLen +
605+
bytesToHex(buf, offset, length);
606+
LOG.warn(msg);
607+
throw new IllegalArgumentException(msg);
580608
}
581609
pos += qualifierLen;
582610
// check the timestamp
583611
if (pos + Bytes.SIZEOF_LONG > endOffset) {
584-
throw new IllegalArgumentException(
585-
"Overflow when reading timestamp at position=" + pos + bytesToHex(buf, offset, length));
612+
String msg =
613+
"Overflow when reading timestamp at position=" + pos + bytesToHex(buf, offset, length);
614+
LOG.warn(msg);
615+
throw new IllegalArgumentException(msg);
586616
}
587617
long timestamp = Bytes.toLong(buf, pos, Bytes.SIZEOF_LONG);
588618
if (timestamp < 0) {
589-
throw new IllegalArgumentException(
590-
"Timestamp cannot be negative, ts=" + timestamp + bytesToHex(buf, offset, length));
619+
String msg =
620+
"Timestamp cannot be negative, ts=" + timestamp + bytesToHex(buf, offset, length);
621+
LOG.warn(msg);
622+
throw new IllegalArgumentException(msg);
591623
}
592624
pos += Bytes.SIZEOF_LONG;
593625
// check the type
594626
if (pos + Bytes.SIZEOF_BYTE > endOffset) {
595-
throw new IllegalArgumentException(
596-
"Overflow when reading type at position=" + pos + bytesToHex(buf, offset, length));
627+
String msg =
628+
"Overflow when reading type at position=" + pos + bytesToHex(buf, offset, length);
629+
LOG.warn(msg);
630+
throw new IllegalArgumentException(msg);
597631
}
598632
byte type = buf[pos];
599633
if (!Type.isValidType(type)) {
600-
throw new IllegalArgumentException(
601-
"Invalid type in KeyValue, type=" + type + bytesToHex(buf, offset, length));
634+
String msg = "Invalid type in KeyValue, type=" + type + bytesToHex(buf, offset, length);
635+
LOG.warn(msg);
636+
throw new IllegalArgumentException(msg);
602637
}
603638
pos += Bytes.SIZEOF_BYTE;
604639
// check the value
605640
if (pos + valLen > endOffset) {
606-
throw new IllegalArgumentException(
607-
"Overflow when reading value part at position=" + pos + bytesToHex(buf, offset, length));
641+
String msg =
642+
"Overflow when reading value part at position=" + pos + bytesToHex(buf, offset, length);
643+
LOG.warn(msg);
644+
throw new IllegalArgumentException(msg);
608645
}
609646
pos += valLen;
610647
// check the tags
@@ -613,39 +650,55 @@ static void checkKeyValueBytes(byte[] buf, int offset, int length, boolean withT
613650
// withTags is true but no tag in the cell.
614651
return;
615652
}
616-
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
617-
throw new IllegalArgumentException("Overflow when reading tags length at position=" + pos
618-
+ bytesToHex(buf, offset, length));
619-
}
620-
short tagsLen = Bytes.toShort(buf, pos);
621-
pos += Bytes.SIZEOF_SHORT;
622-
if (tagsLen < 0 || pos + tagsLen > endOffset) {
623-
throw new IllegalArgumentException("Invalid tags length in KeyValue at position="
624-
+ (pos - Bytes.SIZEOF_SHORT) + bytesToHex(buf, offset, length));
625-
}
626-
int tagsEndOffset = pos + tagsLen;
627-
for (; pos < tagsEndOffset;) {
628-
if (pos + Tag.TAG_LENGTH_SIZE > endOffset) {
629-
throw new IllegalArgumentException("Overflow when reading tag length at position=" + pos
630-
+ bytesToHex(buf, offset, length));
631-
}
632-
short tagLen = Bytes.toShort(buf, pos);
633-
pos += Tag.TAG_LENGTH_SIZE;
634-
// tagLen contains one byte tag type, so must be not less than 1.
635-
if (tagLen < 1 || pos + tagLen > endOffset) {
636-
throw new IllegalArgumentException(
637-
"Invalid tag length at position=" + (pos - Tag.TAG_LENGTH_SIZE) + ", tagLength="
638-
+ tagLen + bytesToHex(buf, offset, length));
639-
}
640-
pos += tagLen;
641-
}
653+
pos = checkKeyValueTagBytes(buf, offset, length, pos, endOffset);
642654
}
643655
if (pos != endOffset) {
644-
throw new IllegalArgumentException("Some redundant bytes in KeyValue's buffer, startOffset="
645-
+ pos + ", endOffset=" + endOffset + bytesToHex(buf, offset, length));
656+
String msg = "Some redundant bytes in KeyValue's buffer, startOffset=" + pos + ", endOffset="
657+
+ endOffset + bytesToHex(buf, offset, length);
658+
LOG.warn(msg);
659+
throw new IllegalArgumentException(msg);
646660
}
647661
}
648662

663+
private static int checkKeyValueTagBytes(byte[] buf, int offset, int length, int pos,
664+
int endOffset) {
665+
if (pos + Bytes.SIZEOF_SHORT > endOffset) {
666+
String msg = "Overflow when reading tags length at position=" + pos +
667+
bytesToHex(buf, offset, length);
668+
LOG.warn(msg);
669+
throw new IllegalArgumentException(msg);
670+
}
671+
short tagsLen = Bytes.toShort(buf, pos);
672+
pos += Bytes.SIZEOF_SHORT;
673+
if (tagsLen < 0 || pos + tagsLen > endOffset) {
674+
String msg = "Invalid tags length in KeyValue at position=" + (pos - Bytes.SIZEOF_SHORT)
675+
+ bytesToHex(buf, offset, length);
676+
LOG.warn(msg);
677+
throw new IllegalArgumentException(msg);
678+
}
679+
int tagsEndOffset = pos + tagsLen;
680+
for (; pos < tagsEndOffset;) {
681+
if (pos + Tag.TAG_LENGTH_SIZE > endOffset) {
682+
String msg = "Overflow when reading tag length at position=" + pos +
683+
bytesToHex(buf, offset, length);
684+
LOG.warn(msg);
685+
throw new IllegalArgumentException(msg);
686+
}
687+
short tagLen = Bytes.toShort(buf, pos);
688+
pos += Tag.TAG_LENGTH_SIZE;
689+
// tagLen contains one byte tag type, so must be not less than 1.
690+
if (tagLen < 1 || pos + tagLen > endOffset) {
691+
String msg =
692+
"Invalid tag length at position=" + (pos - Tag.TAG_LENGTH_SIZE) + ", tagLength="
693+
+ tagLen + bytesToHex(buf, offset, length);
694+
LOG.warn(msg);
695+
throw new IllegalArgumentException(msg);
696+
}
697+
pos += tagLen;
698+
}
699+
return pos;
700+
}
701+
649702
/**
650703
* Create a KeyValue reading from the raw InputStream. Named
651704
* <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
@@ -331,9 +331,7 @@ protected boolean readNext(Entry entry) throws IOException {
331331
// OriginalPosition might be < 0 on local fs; if so, it is useless to us.
332332
long originalPosition = this.inputStream.getPos();
333333
if (trailerPresent && originalPosition > 0 && originalPosition == this.walEditsStopOffset) {
334-
if (LOG.isTraceEnabled()) {
335-
LOG.trace("Reached end of expected edits area at offset " + originalPosition);
336-
}
334+
LOG.trace("Reached end of expected edits area at offset {}", originalPosition);
337335
return false;
338336
}
339337
WALKey.Builder builder = WALKey.newBuilder();
@@ -371,10 +369,8 @@ protected boolean readNext(Entry entry) throws IOException {
371369
WALKey walKey = builder.build();
372370
entry.getKey().readFieldsFromPb(walKey, this.byteStringUncompressor);
373371
if (!walKey.hasFollowingKvCount() || 0 == walKey.getFollowingKvCount()) {
374-
if (LOG.isTraceEnabled()) {
375-
LOG.trace("WALKey has no KVs that follow it; trying the next one. current offset=" +
376-
this.inputStream.getPos());
377-
}
372+
LOG.trace("WALKey has no KVs that follow it; trying the next one. current offset={}",
373+
this.inputStream.getPos());
378374
seekOnFs(originalPosition);
379375
return false;
380376
}
@@ -391,9 +387,7 @@ protected boolean readNext(Entry entry) throws IOException {
391387
try {
392388
posAfterStr = this.inputStream.getPos() + "";
393389
} catch (Throwable t) {
394-
if (LOG.isTraceEnabled()) {
395-
LOG.trace("Error getting pos for error message - ignoring", t);
396-
}
390+
LOG.trace("Error getting pos for error message - ignoring", t);
397391
}
398392
String message = " while reading " + expectedCells + " WAL KVs; started reading at "
399393
+ posBefore + " and read up to " + posAfterStr;
@@ -410,27 +404,21 @@ protected boolean readNext(Entry entry) throws IOException {
410404
} catch (EOFException eof) {
411405
// If originalPosition is < 0, it is rubbish and we cannot use it (probably local fs)
412406
if (originalPosition < 0) {
413-
if (LOG.isTraceEnabled()) {
414-
LOG.trace("Encountered a malformed edit, but can't seek back to last good position "
415-
+ "because originalPosition is negative. last offset="
416-
+ this.inputStream.getPos(), eof);
417-
}
407+
LOG.warn("Encountered a malformed edit, but can't seek back to last good position "
408+
+ "because originalPosition is negative. last offset={}",
409+
this.inputStream.getPos(), eof);
418410
throw eof;
419411
}
420412
// If stuck at the same place and we got and exception, lets go back at the beginning.
421413
if (inputStream.getPos() == originalPosition && resetPosition) {
422-
if (LOG.isTraceEnabled()) {
423-
LOG.trace("Encountered a malformed edit, seeking to the beginning of the WAL since "
424-
+ "current position and original position match at " + originalPosition);
425-
}
414+
LOG.warn("Encountered a malformed edit, seeking to the beginning of the WAL since "
415+
+ "current position and original position match at {}", originalPosition);
426416
seekOnFs(0);
427417
} else {
428418
// Else restore our position to original location in hope that next time through we will
429419
// read successfully.
430-
if (LOG.isTraceEnabled()) {
431-
LOG.trace("Encountered a malformed edit, seeking back to last good position in file, "
432-
+ "from " + inputStream.getPos()+" to " + originalPosition, eof);
433-
}
420+
LOG.warn("Encountered a malformed edit, seeking back to last good position in file, "
421+
+ "from {} to {}", inputStream.getPos(), originalPosition, eof);
434422
seekOnFs(originalPosition);
435423
}
436424
return false;

0 commit comments

Comments
 (0)