Skip to content

Commit

Permalink
Add no-value key handling only for form body
Browse files Browse the repository at this point in the history 
Motivation:

This is a fix for issue netty#13981 that reports a changed behaviour of HttpPostStandardRequestDecoder after this PR - netty#13908

Because HttpPostStandardRequestDecoder changed the contract, some code implementations relying on certain parsing are failing

Modification:

This PR makes sure, that the edge case handling for form data happenes only when the content is in fact form data

Result:

Fixes netty#13981
  • Loading branch information
@gniadeck
gniadeck committed Apr 23, 2024
1 parent 93caaeb commit e6f3135
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 14 deletions.
14 changes: 11 additions & 3 deletions ...p/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.HttpContent;
import io.netty.handler.codec.http.HttpRequest;
Expand Down Expand Up @@ -471,7 +470,7 @@ private void parseBodyAttributesStandard() {
charset);
currentAttribute = factory.createAttribute(request, key);
firstpos = currentpos;
} else if (read == '&' || (isLastChunk && !undecodedChunk.isReadable())) { // special empty FIELD
} else if (read == '&' || (isLastChunk && !undecodedChunk.isReadable() && hasFormBody())) { // special empty FIELD
currentStatus = MultiPartStatus.DISPOSITION;
ampersandpos = read == '&' ? currentpos - 1 : currentpos;
String key = decodeAttribute(
Expand Down Expand Up @@ -597,7 +596,7 @@ private void parseBodyAttributes() {
charset);
currentAttribute = factory.createAttribute(request, key);
firstpos = currentpos;
} else if (read == '&' || (isLastChunk && !undecodedChunk.isReadable())) { // special empty FIELD
} else if (read == '&' || (isLastChunk && !undecodedChunk.isReadable() && hasFormBody())) { // special empty FIELD
currentStatus = MultiPartStatus.DISPOSITION;
ampersandpos = read == '&' ? currentpos - 1 : currentpos;
String key = decodeAttribute(
Expand Down Expand Up @@ -783,6 +782,15 @@ public void removeHttpDataFromClean(InterfaceHttpData data) {
factory.removeHttpDataFromClean(request, data);
}

/**
* Check if request has headers indicating that it contains form body
*/
private boolean hasFormBody() {
String contentHeader = request.headers().get("Content-Type");
if(contentHeader == null) return false;
return contentHeader.equals("application/x-www-form-urlencoded") || contentHeader.equals("multipart/form-data");
}

private static final class UrlEncodedDetector implements ByteProcessor {
@Override
public boolean process(byte value) throws Exception {
Expand Down
52 changes: 41 additions & 11 deletions ...c/test/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,11 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.handler.codec.http.DefaultHttpContent;
import io.netty.handler.codec.http.DefaultHttpRequest;
import io.netty.handler.codec.http.DefaultLastHttpContent;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpRequest;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.handler.codec.http.*;
import io.netty.util.CharsetUtil;
import org.junit.jupiter.api.Test;

import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.headersFactory;
import static org.junit.jupiter.api.Assertions.*;

class HttpPostStandardRequestDecoderTest {
Expand All @@ -52,7 +47,8 @@ void testDecodeAttributes() {
void testDecodeSingleAttributeWithNoValue() {
String requestBody = "key1";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
headersFactory().newHeaders().add("Content-Type", "application/x-www-form-urlencoded"));

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
Expand All @@ -68,7 +64,8 @@ void testDecodeSingleAttributeWithNoValue() {
void testDecodeSingleAttributeWithNoValueEmptyLast() {
String requestBody = "key1";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
headersFactory().newHeaders().add("Content-Type", "application/x-www-form-urlencoded"));

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
Expand All @@ -86,7 +83,8 @@ void testDecodeSingleAttributeWithNoValueEmptyLast() {
void testDecodeEndAttributeWithNoValue() {
String requestBody = "key1=value1&key2";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
headersFactory().newHeaders().add("Content-Type", "application/x-www-form-urlencoded"));

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
Expand All @@ -99,6 +97,37 @@ void testDecodeEndAttributeWithNoValue() {
decoder.destroy();
}

@Test
void testDecodeJsonAttributeAsEmpty() {
String requestBody = "{\"iAm\": \" a JSON!\"}";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
headersFactory().newHeaders().add("Content-Type", "application/json"));

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
DefaultHttpContent httpContent = new DefaultLastHttpContent(buf);
decoder.offer(httpContent);

assertEquals(0, decoder.getBodyHttpDatas().size());
decoder.destroy();
}

@Test
void testDecodeJsonAttributeAsEmptyAndNoHeaders() {
String requestBody = "{\"iAm\": \" a JSON!\"}";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
DefaultHttpContent httpContent = new DefaultLastHttpContent(buf);
decoder.offer(httpContent);

assertEquals(0, decoder.getBodyHttpDatas().size());
decoder.destroy();
}

@Test
void testDecodeStartAttributeWithNoValue() {
String requestBody = "key1&key2=value2";
Expand All @@ -120,7 +149,8 @@ void testDecodeStartAttributeWithNoValue() {
void testDecodeMultipleAttributesWithNoValue() {
String requestBody = "key1&key2&key3";

HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload");
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/upload",
headersFactory().newHeaders().add("Content-Type", "application/x-www-form-urlencoded"));

HttpPostStandardRequestDecoder decoder = new HttpPostStandardRequestDecoder(httpDiskDataFactory(), request);
ByteBuf buf = Unpooled.wrappedBuffer(requestBody.getBytes(CharsetUtil.UTF_8));
Expand Down

0 comments on commit e6f3135

Please sign in to comment.