Fix a bug where ContentPreviewer unexpectly releases an HttpData

[ikhoon]

Motivation:

When creating a ContentPreviewerFactory withtout specifying maxLength(...),
A created ContentPreviewer from the factory will have zero maxLength.

ContentPreviewerFactory factory =
    ContentPreviewerFactory.builder()
                            .text((unused1, unused2) -> true)
                            .build();

• The zero maxLength makes length zero always in the ContentPreviewer.
  

  armeria/core/src/main/java/com/linecorp/armeria/common/logging/LengthLimitingContentPreviewer.java

  Line 52 in e95e07b

  inflatedMaxLength = inflateMaxLength(maxLength, charset);

• And also length could be set to 0 according to consumed content length.
  

  armeria/core/src/main/java/com/linecorp/armeria/common/logging/LengthLimitingContentPreviewer.java

  Line 69 in e95e07b

  final int length = Math.min(inflatedMaxLength - aggregatedLength, data.length());

If the length is equal to 0:

• retainedSlice(...) will return an empty ByteBuf.
  

  armeria/core/src/main/java/com/linecorp/armeria/common/logging/LengthLimitingContentPreviewer.java

  Lines 78 to 88 in e95e07b

  	private static ByteBuf duplicateData(HttpData httpData, int length) {
  	if (httpData instanceof ByteBufHolder) {
  	final ByteBuf content = ((ByteBufHolder) httpData).content();
  	if (content.readableBytes() == length) {
  	return content.retainedDuplicate();
  	}
  	return content.retainedSlice(content.readerIndex(), length);
  	} else {
  	return Unpooled.wrappedBuffer(httpData.array(), 0, length);
  	}
  	}

• The empty ByteBuf is not readable.
• The unreadable ByteBuf be released unexpectedly when wrapped by Unpooled.wrappedBuffer(emptyBuf).
  https://github.com/netty/netty/blob/5a08dc0d9aeafe3b4e242e3b7722bfbd38acbbb2/buffer/src/main/java/io/netty/buffer/Unpooled.java#L317
• The released ByteBuf will cause an IllegalReferenceCountException
  when it is consumed by HttpResponseDecoder

Modifications:

• Set the default value of the max length of content preview to 32.
• Do not duplicate if preview content length is 0

Result:

You can not see IllegalReferenceCountException anymore
when you log a preview content using ContentPreview{Client, Service}.

[minwoox]

Thanks!

[codecov]

Codecov Report

Merging #2905 into master will increase coverage by 0.01%.
The diff coverage is 69.56%.

@@             Coverage Diff              @@
##             master    #2905      +/-   ##
============================================
+ Coverage     72.88%   72.90%   +0.01%     
- Complexity    12179    12190      +11     
============================================
  Files          1069     1070       +1     
  Lines         47389    47408      +19     
  Branches       5944     5947       +3     
============================================
+ Hits          34541    34561      +20     
- Misses         9819     9821       +2     
+ Partials       3029     3026       -3     

Impacted Files	Coverage Δ	Complexity Δ
...armeria/server/AnnotatedServiceBindingBuilder.java	77.77% <0.00%> (-2.56%)	15.00 <0.00> (ø)
.../linecorp/armeria/server/ServiceConfigBuilder.java	63.88% <0.00%> (-1.83%)	13.00 <0.00> (ø)
...ver/VirtualHostAnnotatedServiceBindingBuilder.java	60.31% <0.00%> (-1.98%)	11.00 <0.00> (ø)
...meria/server/VirtualHostServiceBindingBuilder.java	41.46% <0.00%> (-1.04%)	13.00 <0.00> (ø)
...common/logging/LengthLimitingContentPreviewer.java	93.75% <75.00%> (-3.03%)	14.00 <0.00> (+1.00)	⬇️
...ain/java/com/linecorp/armeria/client/Endpoint.java	90.78% <100.00%> (+0.24%)	84.00 <2.00> (+2.00)
...common/logging/ContentPreviewerFactoryBuilder.java	71.05% <100.00%> (+8.89%)	14.00 <0.00> (+1.00)
.../armeria/server/AbstractServiceBindingBuilder.java	96.42% <100.00%> (+0.27%)	12.00 <1.00> (+1.00)
...linecorp/armeria/server/ServiceBindingBuilder.java	78.04% <100.00%> (+0.54%)	28.00 <1.00> (+1.00)
.../linecorp/armeria/server/ServiceConfigSetters.java	100.00% <100.00%> (ø)	2.00 <2.00> (?)
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9895168...2777c0a. Read the comment docs.

[trustin]

👍