Repository navigation
Description
set HttpDataFactory to use disk = false like below:
HttpDataFactory factory = new DefaultHttpDataFactory(false, CharsetUtil.UTF_8);
and then create an HttpPostRequestDecoder instance:
HttpPostRequestDecoder httpDecoder = new HttpPostRequestDecoder(factory, httpRequest);
after decoder is destroyed the reference count of httpRequest is incorrect.
in the method createAttribute of DefaultHttpDataFactory, MemoryAttribute is not added to HttpData list:
@Override
public Attribute createAttribute(HttpRequest request, String name, long definedSize) {
if (useDisk) {
Attribute attribute = new DiskAttribute(name, definedSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, definedSize, minSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
MemoryAttribute attribute = new MemoryAttribute(name, definedSize);
attribute.setMaxSize(maxSize);
return attribute;
}
so on destroying
@Override
public void cleanAllHttpData() {
Iterator<Entry<HttpRequest, List<HttpData>>> i = requestFileDeleteMap.entrySet().iterator();
while (i.hasNext()) {
Entry<HttpRequest, List<HttpData>> e = i.next();
// Calling i.remove() here will cause "java.lang.IllegalStateException: Entry was removed"
// at e.getValue() below
List<HttpData> list = e.getValue();
for (HttpData data : list) {
data.release();
}
i.remove();
}
}
the MemoryAttributes created above are not released.
I'm not sure if it is a bug.
Expected behavior
the reference count of httpRequest before decoding should equals to the one after decoder is destroyed.
Actual behavior
if the original reference count of http request is 1, and the http request contains 2 fields , then after destroying the reference count is 3 !
Steps to reproduce
Minimal yet complete reproducer code (or URL to code)
send a form :
public void testSubmitForm() throws IOException {
String boundary = "---------------------------ABCDEFGHIJKLMNOPQRSTUVWXYZ";
URL url = new URL("http://127.0.0.1:1084/test");
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
connection.setConnectTimeout(5000);
connection.setReadTimeout(30000);
connection.setDoOutput(true);
connection.setDoInput(true);
connection.setUseCaches(false);
connection.setRequestMethod("POST");
connection.setRequestProperty("Connection", "Keep-Alive");
connection.setRequestProperty("User-Agent", "Mozilla/5.0 (Windows; U; Windows NT 6.1; zh-CN; rv:1.9.2.6)");
connection.setRequestProperty("Content-Type", "multipart/form-data; boundary=" + boundary);
try (OutputStream out = new DataOutputStream(connection.getOutputStream())) {
out.write(("\r\n--" + boundary + "\r\n").getBytes(StandardCharsets.UTF_8));
out.write("Content-Disposition: form-data; name=\"username\"\r\n\r\n".getBytes(StandardCharsets.UTF_8));
out.write("chinnsii".getBytes(StandardCharsets.UTF_8));
out.write(("\r\n--" + boundary + "\r\n").getBytes(StandardCharsets.UTF_8));
out.write("Content-Disposition: form-data; name=\"password\"\r\n\r\n".getBytes(StandardCharsets.UTF_8));
out.write("chinnsii".getBytes(StandardCharsets.UTF_8));
byte[] endData = ("\r\n--" + boundary + "--\r\n").getBytes();
out.write(endData);
out.flush();
}
try (BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
String line;
while ((line = reader.readLine()) != null) {
System.out.println(line);
}
}
}
decode to a hash map
Map<String, Object> convert(FullHttpRequest httpRequest) throws IOException {
HttpDataFactory factory = new DefaultHttpDataFactory(false, CharsetUtil.UTF_8);
HttpPostRequestDecoder httpDecoder = new HttpPostRequestDecoder(factory, httpRequest);
try {
httpDecoder.setDiscardThreshold(0);
Map<String, Object> params = new HashMap<>();
List<InterfaceHttpData> InterfaceHttpDataList = httpDecoder.getBodyHttpDatas();
for (InterfaceHttpData data : InterfaceHttpDataList) {
if (data != null) {
if (InterfaceHttpData.HttpDataType.FileUpload == data.getHttpDataType()) {
FileUpload fileUpload = (FileUpload) data;
byte[] bytes = fileUpload.get();
if (!fileUpload.isInMemory()) {
fileUpload.delete();
}
params.put(data.getName(), bytes);
}
if (data.getHttpDataType() == InterfaceHttpData.HttpDataType.Attribute) {
Attribute attribute = (Attribute) data;
params.put(attribute.getName(), attribute.getValue());
}
}
}
return params;
} finally {
System.out.println("74------------->" + httpRequest.refCnt());
httpDecoder.destroy();
System.out.println("76------------>" + httpRequest.refCnt());
}
}
where 74--------------> is 4, and 76----------------->is 3
Netty version
4.1.51.Final
JVM version (e.g. java -version)
1.8
OS version (e.g. uname -a)
MAC
Activity
aepedraza commentedon Mar 24, 2021
Hi, I'm trying to reproduce your code in a unit test but I'm not finding
ContentConverterExceptionnorUploadedFile. My version is4.1.61.Final-SNAPSHOT(currently the last in branch 4.1) Are those some "theoretical classes" or is this in a previous version?Thanks in advance for your time
Disclaimer: This may be my first contribution to the project. Please forgive my newbie questions
devilyard commentedon Mar 24, 2021
thank you for replying. sorry for my posting, UploadedFile and ContentConverterException is my local definition, I will modify the code.
aepedraza commentedon Mar 27, 2021
Hi @devilyard I'm terribly sorry for not being able to read your code correctly, but
filevariable is not being resolved inconvert(), did you meantbytesdeclared before and not being used after?fredericBregier commentedon Mar 27, 2021
The API says:
So MemoryAttribute is not concerned by this method.
Just for note, for both Memory or Disk or Mixed mode factories, the release has to be done as:
jameskleeh commentedon Apr 6, 2021
Relates to #8640
fredericBregier commentedon Apr 6, 2021
@jameskleeh Yes and no... It is the current behavior.
I give the right way to clean up, whatever Memory, Disk or Mixed mode in my last comment.
aepedraza commentedon Apr 7, 2021
@fredericBregier So, should this issue be discarded?
fredericBregier commentedon Apr 7, 2021
@aepedraza I think so.
Note however on your example code:
I would recommend to free Memory, Mixed, Disk based factory as the following, once all stuffs are done.
chrisvest commentedon Apr 8, 2021
@devilyard (ignore my previous comment – the web page was out of date when I posted) Does Frederics help solve the issue you are seeing? If so, then I think we can close this issue.
devilyard commentedon Apr 8, 2021
yes, i tried the latest 4. 1. 62 version, the issue is fixed