diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractHttpData.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractHttpData.java index c9448f9..cc91071 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractHttpData.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractHttpData.java @@ -83,7 +83,11 @@ public boolean isCompleted() { } protected void setCompleted() { - completed = true; + setCompleted(true); + } + + protected void setCompleted(boolean completed) { + this.completed = completed; } @Override diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractMixedHttpData.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractMixedHttpData.java new file mode 100644 index 0000000..d6aa461 --- /dev/null +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/AbstractMixedHttpData.java @@ -0,0 +1,279 @@ +/* + * Copyright 2022 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.contrib.handler.codec.http.multipart; + +import io.netty.buffer.ByteBuf; +import io.netty.util.AbstractReferenceCounted; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.Charset; + +abstract class AbstractMixedHttpData extends AbstractReferenceCounted implements HttpData { + final String baseDir; + final boolean deleteOnExit; + D wrapped; + + private final long limitSize; + + AbstractMixedHttpData(long limitSize, String baseDir, boolean deleteOnExit, D initial) { + this.limitSize = limitSize; + this.wrapped = initial; + this.baseDir = baseDir; + this.deleteOnExit = deleteOnExit; + } + + abstract D makeDiskData(); + + @Override + public long getMaxSize() { + return wrapped.getMaxSize(); + } + + @Override + public void setMaxSize(long maxSize) { + wrapped.setMaxSize(maxSize); + } + + @Override + public ByteBuf content() { + return wrapped.content(); + } + + @Override + public void checkSize(long newSize) throws IOException { + wrapped.checkSize(newSize); + } + + @Override + public long definedLength() { + return wrapped.definedLength(); + } + + @Override + public Charset getCharset() { + return wrapped.getCharset(); + } + + @Override + public String getName() { + return wrapped.getName(); + } + + @Override + public void addContent(ByteBuf buffer, boolean last) throws IOException { + if (wrapped instanceof AbstractMemoryHttpData) { + try { + checkSize(wrapped.length() + buffer.readableBytes()); + if (wrapped.length() + buffer.readableBytes() > limitSize) { + D diskData = makeDiskData(); + ByteBuf data = ((AbstractMemoryHttpData) wrapped).getByteBuf(); + if (data != null && data.isReadable()) { + diskData.addContent(data.retain(), false); + } + wrapped.release(); + wrapped = diskData; + } + } catch (IOException e) { + buffer.release(); + throw e; + } + } + wrapped.addContent(buffer, last); + } + + @Override + protected void deallocate() { + delete(); + } + + @Override + public void delete() { + wrapped.delete(); + } + + @Override + public byte[] get() throws IOException { + return wrapped.get(); + } + + @Override + public ByteBuf getByteBuf() throws IOException { + return wrapped.getByteBuf(); + } + + @Override + public String getString() throws IOException { + return wrapped.getString(); + } + + @Override + public String getString(Charset encoding) throws IOException { + return wrapped.getString(encoding); + } + + @Override + public boolean isInMemory() { + return wrapped.isInMemory(); + } + + @Override + public long length() { + return wrapped.length(); + } + + @Override + public boolean renameTo(File dest) throws IOException { + return wrapped.renameTo(dest); + } + + @Override + public void setCharset(Charset charset) { + wrapped.setCharset(charset); + } + + @Override + public void setContent(ByteBuf buffer) throws IOException { + try { + checkSize(buffer.readableBytes()); + } catch (IOException e) { + buffer.release(); + throw e; + } + if (buffer.readableBytes() > limitSize) { + if (wrapped instanceof AbstractMemoryHttpData) { + // change to Disk + wrapped.release(); + wrapped = makeDiskData(); + } + } + wrapped.setContent(buffer); + } + + @Override + public void setContent(File file) throws IOException { + checkSize(file.length()); + if (file.length() > limitSize) { + if (wrapped instanceof AbstractMemoryHttpData) { + // change to Disk + wrapped.release(); + wrapped = makeDiskData(); + } + } + wrapped.setContent(file); + } + + @Override + public void setContent(InputStream inputStream) throws IOException { + if (wrapped instanceof AbstractMemoryHttpData) { + // change to Disk even if we don't know the size + wrapped.release(); + wrapped = makeDiskData(); + } + wrapped.setContent(inputStream); + } + + @Override + public boolean isCompleted() { + return wrapped.isCompleted(); + } + + @Override + public HttpDataType getHttpDataType() { + return wrapped.getHttpDataType(); + } + + @Override + public int hashCode() { + return wrapped.hashCode(); + } + + @Override + public boolean equals(Object obj) { + return wrapped.equals(obj); + } + + @Override + public int compareTo(InterfaceHttpData o) { + return wrapped.compareTo(o); + } + + @Override + public String toString() { + return "Mixed: " + wrapped; + } + + @Override + public ByteBuf getChunk(int length) throws IOException { + return wrapped.getChunk(length); + } + + @Override + public File getFile() throws IOException { + return wrapped.getFile(); + } + + @SuppressWarnings("unchecked") + @Override + public D copy() { + return (D) wrapped.copy(); + } + + @SuppressWarnings("unchecked") + @Override + public D duplicate() { + return (D) wrapped.duplicate(); + } + + @SuppressWarnings("unchecked") + @Override + public D retainedDuplicate() { + return (D) wrapped.retainedDuplicate(); + } + + @SuppressWarnings("unchecked") + @Override + public D replace(ByteBuf content) { + return (D) wrapped.replace(content); + } + + @SuppressWarnings("unchecked") + @Override + public D touch() { + wrapped.touch(); + return (D) this; + } + + @SuppressWarnings("unchecked") + @Override + public D touch(Object hint) { + wrapped.touch(hint); + return (D) this; + } + + @SuppressWarnings("unchecked") + @Override + public D retain() { + return (D) super.retain(); + } + + @SuppressWarnings("unchecked") + @Override + public D retain(int increment) { + return (D) super.retain(increment); + } +} diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskAttribute.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskAttribute.java index 2cdcfd6..7698f19 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskAttribute.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskAttribute.java @@ -242,6 +242,7 @@ public Attribute replace(ByteBuf content) { throw new ChannelException(e); } } + attr.setCompleted(isCompleted()); return attr; } diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskFileUpload.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskFileUpload.java index 105bb8f..e754ddb 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskFileUpload.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/DiskFileUpload.java @@ -210,6 +210,7 @@ public FileUpload replace(ByteBuf content) { throw new ChannelException(e); } } + upload.setCompleted(isCompleted()); return upload; } diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java index c953302..3be78c8 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java @@ -431,9 +431,15 @@ private void parseBodyAttributesStandard() { ampersandpos = currentpos - 1; String key = decodeAttribute( undecodedChunk.toString(firstpos, ampersandpos - firstpos, charset), charset); - currentAttribute = factory.createAttribute(request, key); - currentAttribute.setValue(""); // empty - addHttpData(currentAttribute); + // Some weird request bodies start with an '&' character, eg: &name=J&age=17. + // In that case, key would be "", will get exception: + // java.lang.IllegalArgumentException: Param 'name' must not be empty; + // Just check and skip empty key. + if (!key.isEmpty()) { + currentAttribute = factory.createAttribute(request, key); + currentAttribute.setValue(""); // empty + addHttpData(currentAttribute); + } currentAttribute = null; firstpos = currentpos; contRead = true; @@ -551,9 +557,15 @@ private void parseBodyAttributes() { ampersandpos = currentpos - 1; String key = decodeAttribute( undecodedChunk.toString(firstpos, ampersandpos - firstpos, charset), charset); - currentAttribute = factory.createAttribute(request, key); - currentAttribute.setValue(""); // empty - addHttpData(currentAttribute); + // Some weird request bodies start with an '&' char, eg: &name=J&age=17. + // In that case, key would be "", will get exception: + // java.lang.IllegalArgumentException: Param 'name' must not be empty; + // Just check and skip empty key. + if (!key.isEmpty()) { + currentAttribute = factory.createAttribute(request, key); + currentAttribute.setValue(""); // empty + addHttpData(currentAttribute); + } currentAttribute = null; firstpos = currentpos; contRead = true; diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryAttribute.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryAttribute.java index 66d13cb..70b5b88 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryAttribute.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryAttribute.java @@ -167,6 +167,7 @@ public Attribute replace(ByteBuf content) { throw new ChannelException(e); } } + attr.setCompleted(isCompleted()); return attr; } diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryFileUpload.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryFileUpload.java index 5cffa7b..4809a14 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryFileUpload.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MemoryFileUpload.java @@ -154,11 +154,11 @@ public FileUpload replace(ByteBuf content) { if (content != null) { try { upload.setContent(content); - return upload; } catch (IOException e) { throw new ChannelException(e); } } + upload.setCompleted(isCompleted()); return upload; } diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedAttribute.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedAttribute.java index fbbef8f..8ec0269 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedAttribute.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedAttribute.java @@ -18,22 +18,13 @@ import io.netty.buffer.ByteBuf; import io.netty.handler.codec.http.HttpConstants; -import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.charset.Charset; /** * Mixed implementation using both in Memory and in File with a limit of size */ -public class MixedAttribute implements Attribute { - private final String baseDir; - private final boolean deleteOnExit; - private Attribute attribute; - - private final long limitSize; - private long maxSize = DefaultHttpDataFactory.MAXSIZE; - +public class MixedAttribute extends AbstractMixedHttpData implements Attribute { public MixedAttribute(String name, long limitSize) { this(name, limitSize, HttpConstants.DEFAULT_CHARSET); } @@ -47,10 +38,7 @@ public MixedAttribute(String name, long limitSize, Charset charset) { } public MixedAttribute(String name, long limitSize, Charset charset, String baseDir, boolean deleteOnExit) { - this.limitSize = limitSize; - attribute = new MemoryAttribute(name, charset); - this.baseDir = baseDir; - this.deleteOnExit = deleteOnExit; + this(name, 0, limitSize, charset, baseDir, deleteOnExit); } public MixedAttribute(String name, long definedSize, long limitSize, Charset charset) { @@ -60,10 +48,8 @@ public MixedAttribute(String name, long definedSize, long limitSize, Charset cha public MixedAttribute(String name, long definedSize, long limitSize, Charset charset, String baseDir, boolean deleteOnExit) { - this.limitSize = limitSize; - attribute = new MemoryAttribute(name, definedSize, charset); - this.baseDir = baseDir; - this.deleteOnExit = deleteOnExit; + super(limitSize, baseDir, deleteOnExit, + new MemoryAttribute(name, definedSize, charset)); } public MixedAttribute(String name, String value, long limitSize) { @@ -76,284 +62,96 @@ public MixedAttribute(String name, String value, long limitSize, Charset charset DiskAttribute.baseDirectory, DiskFileUpload.deleteOnExitTemporaryFile); } - public MixedAttribute(String name, String value, long limitSize, Charset charset, - String baseDir, boolean deleteOnExit) { - this.limitSize = limitSize; - if (value.length() > this.limitSize) { + private static Attribute makeInitialAttributeFromValue(String name, String value, long limitSize, Charset charset, + String baseDir, boolean deleteOnExit) { + if (value.length() > limitSize) { try { - attribute = new DiskAttribute(name, value, charset, baseDir, deleteOnExit); + return new DiskAttribute(name, value, charset, baseDir, deleteOnExit); } catch (IOException e) { // revert to Memory mode try { - attribute = new MemoryAttribute(name, value, charset); + return new MemoryAttribute(name, value, charset); } catch (IOException ignore) { throw new IllegalArgumentException(e); } } } else { try { - attribute = new MemoryAttribute(name, value, charset); + return new MemoryAttribute(name, value, charset); } catch (IOException e) { throw new IllegalArgumentException(e); } } - this.baseDir = baseDir; - this.deleteOnExit = deleteOnExit; - } - - @Override - public long getMaxSize() { - return maxSize; - } - - @Override - public void setMaxSize(long maxSize) { - this.maxSize = maxSize; - attribute.setMaxSize(maxSize); - } - - @Override - public void checkSize(long newSize) throws IOException { - if (maxSize >= 0 && newSize > maxSize) { - throw new IOException("Size exceed allowed maximum capacity"); - } - } - - @Override - public void addContent(ByteBuf buffer, boolean last) throws IOException { - if (attribute instanceof MemoryAttribute) { - try { - checkSize(attribute.length() + buffer.readableBytes()); - if (attribute.length() + buffer.readableBytes() > limitSize) { - DiskAttribute diskAttribute = new DiskAttribute(attribute - .getName(), attribute.definedLength(), baseDir, deleteOnExit); - diskAttribute.setMaxSize(maxSize); - if (((MemoryAttribute) attribute).getByteBuf() != null) { - diskAttribute.addContent(((MemoryAttribute) attribute) - .getByteBuf(), false); - } - attribute = diskAttribute; - } - } catch (IOException e) { - buffer.release(); - throw e; - } - } - attribute.addContent(buffer, last); - } - - @Override - public void delete() { - attribute.delete(); - } - - @Override - public byte[] get() throws IOException { - return attribute.get(); - } - - @Override - public ByteBuf getByteBuf() throws IOException { - return attribute.getByteBuf(); - } - - @Override - public Charset getCharset() { - return attribute.getCharset(); - } - - @Override - public String getString() throws IOException { - return attribute.getString(); - } - - @Override - public String getString(Charset encoding) throws IOException { - return attribute.getString(encoding); - } - - @Override - public boolean isCompleted() { - return attribute.isCompleted(); - } - - @Override - public boolean isInMemory() { - return attribute.isInMemory(); - } - - @Override - public long length() { - return attribute.length(); - } - - @Override - public long definedLength() { - return attribute.definedLength(); - } - - @Override - public boolean renameTo(File dest) throws IOException { - return attribute.renameTo(dest); - } - - @Override - public void setCharset(Charset charset) { - attribute.setCharset(charset); - } - - @Override - public void setContent(ByteBuf buffer) throws IOException { - try { - checkSize(buffer.readableBytes()); - } catch (IOException e) { - buffer.release(); - throw e; - } - if (buffer.readableBytes() > limitSize) { - if (attribute instanceof MemoryAttribute) { - // change to Disk - attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit); - attribute.setMaxSize(maxSize); - } - } - attribute.setContent(buffer); - } - - @Override - public void setContent(File file) throws IOException { - checkSize(file.length()); - if (file.length() > limitSize) { - if (attribute instanceof MemoryAttribute) { - // change to Disk - attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit); - attribute.setMaxSize(maxSize); - } - } - attribute.setContent(file); - } - - @Override - public void setContent(InputStream inputStream) throws IOException { - if (attribute instanceof MemoryAttribute) { - // change to Disk even if we don't know the size - attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit); - attribute.setMaxSize(maxSize); - } - attribute.setContent(inputStream); - } - - @Override - public HttpDataType getHttpDataType() { - return attribute.getHttpDataType(); - } - - @Override - public String getName() { - return attribute.getName(); - } - - @Override - public int hashCode() { - return attribute.hashCode(); - } - - @Override - public boolean equals(Object obj) { - return attribute.equals(obj); - } - - @Override - public int compareTo(InterfaceHttpData o) { - return attribute.compareTo(o); } - @Override - public String toString() { - return "Mixed: " + attribute; + public MixedAttribute(String name, String value, long limitSize, Charset charset, + String baseDir, boolean deleteOnExit) { + super(limitSize, baseDir, deleteOnExit, + makeInitialAttributeFromValue(name, value, limitSize, charset, baseDir, deleteOnExit)); } @Override public String getValue() throws IOException { - return attribute.getValue(); + return wrapped.getValue(); } @Override public void setValue(String value) throws IOException { - attribute.setValue(value); + wrapped.setValue(value); } @Override - public ByteBuf getChunk(int length) throws IOException { - return attribute.getChunk(length); - } - - @Override - public File getFile() throws IOException { - return attribute.getFile(); + Attribute makeDiskData() { + DiskAttribute diskAttribute = new DiskAttribute(getName(), definedLength(), baseDir, deleteOnExit); + diskAttribute.setMaxSize(getMaxSize()); + return diskAttribute; } @Override public Attribute copy() { - return attribute.copy(); + // for binary compatibility + return super.copy(); } @Override public Attribute duplicate() { - return attribute.duplicate(); - } - - @Override - public Attribute retainedDuplicate() { - return attribute.retainedDuplicate(); + // for binary compatibility + return super.duplicate(); } @Override public Attribute replace(ByteBuf content) { - return attribute.replace(content); - } - - @Override - public ByteBuf content() { - return attribute.content(); - } - - @Override - public int refCnt() { - return attribute.refCnt(); + // for binary compatibility + return super.replace(content); } @Override public Attribute retain() { - attribute.retain(); - return this; + // for binary compatibility + return super.retain(); } @Override public Attribute retain(int increment) { - attribute.retain(increment); - return this; - } - - @Override - public Attribute touch() { - attribute.touch(); - return this; + // for binary compatibility + return super.retain(increment); } @Override - public Attribute touch(Object hint) { - attribute.touch(hint); - return this; + public Attribute retainedDuplicate() { + // for binary compatibility + return super.retainedDuplicate(); } @Override - public boolean release() { - return attribute.release(); + public Attribute touch() { + // for binary compatibility + return super.touch(); } @Override - public boolean release(int decrement) { - return attribute.release(decrement); + public Attribute touch(Object hint) { + // for binary compatibility + return super.touch(hint); } } diff --git a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedFileUpload.java b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedFileUpload.java index a56fa54..f6c35a2 100644 --- a/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedFileUpload.java +++ b/codec-multipart/src/main/java/io/netty/contrib/handler/codec/http/multipart/MixedFileUpload.java @@ -17,26 +17,12 @@ import io.netty.buffer.ByteBuf; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; import java.nio.charset.Charset; /** * Mixed implementation using both in Memory and in File with a limit of size */ -public class MixedFileUpload implements FileUpload { - - private final String baseDir; - - private final boolean deleteOnExit; - - private FileUpload fileUpload; - - private final long limitSize; - - private final long definedSize; - private long maxSize = DefaultHttpDataFactory.MAXSIZE; +public class MixedFileUpload extends AbstractMixedHttpData implements FileUpload { public MixedFileUpload(String name, String filename, String contentType, String contentTransferEncoding, Charset charset, long size, @@ -48,325 +34,97 @@ public MixedFileUpload(String name, String filename, String contentType, public MixedFileUpload(String name, String filename, String contentType, String contentTransferEncoding, Charset charset, long size, long limitSize, String baseDir, boolean deleteOnExit) { - this.limitSize = limitSize; - if (size > this.limitSize) { - fileUpload = new DiskFileUpload(name, filename, contentType, - contentTransferEncoding, charset, size); - } else { - fileUpload = new MemoryFileUpload(name, filename, contentType, - contentTransferEncoding, charset, size); - } - definedSize = size; - this.baseDir = baseDir; - this.deleteOnExit = deleteOnExit; - } - - @Override - public long getMaxSize() { - return maxSize; - } - - @Override - public void setMaxSize(long maxSize) { - this.maxSize = maxSize; - fileUpload.setMaxSize(maxSize); - } - - @Override - public void checkSize(long newSize) throws IOException { - if (maxSize >= 0 && newSize > maxSize) { - throw new IOException("Size exceed allowed maximum capacity"); - } - } - - @Override - public void addContent(ByteBuf buffer, boolean last) - throws IOException { - if (fileUpload instanceof MemoryFileUpload) { - try { - checkSize(fileUpload.length() + buffer.readableBytes()); - if (fileUpload.length() + buffer.readableBytes() > limitSize) { - DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload - .getName(), fileUpload.getFilename(), fileUpload - .getContentType(), fileUpload - .getContentTransferEncoding(), fileUpload.getCharset(), - definedSize, baseDir, deleteOnExit); - diskFileUpload.setMaxSize(maxSize); - ByteBuf data = fileUpload.getByteBuf(); - if (data != null && data.isReadable()) { - diskFileUpload.addContent(data.retain(), false); - } - // release old upload - fileUpload.release(); - - fileUpload = diskFileUpload; - } - } catch (IOException e) { - buffer.release(); - throw e; - } - } - fileUpload.addContent(buffer, last); - } - - @Override - public void delete() { - fileUpload.delete(); - } - - @Override - public byte[] get() throws IOException { - return fileUpload.get(); - } - - @Override - public ByteBuf getByteBuf() throws IOException { - return fileUpload.getByteBuf(); - } - - @Override - public Charset getCharset() { - return fileUpload.getCharset(); - } - - @Override - public String getContentType() { - return fileUpload.getContentType(); + super(limitSize, baseDir, deleteOnExit, + size > limitSize ? + new DiskFileUpload(name, filename, contentType, contentTransferEncoding, charset, size) : + new MemoryFileUpload(name, filename, contentType, contentTransferEncoding, charset, size) + ); } @Override public String getContentTransferEncoding() { - return fileUpload.getContentTransferEncoding(); + return wrapped.getContentTransferEncoding(); } @Override public String getFilename() { - return fileUpload.getFilename(); - } - - @Override - public String getString() throws IOException { - return fileUpload.getString(); - } - - @Override - public String getString(Charset encoding) throws IOException { - return fileUpload.getString(encoding); - } - - @Override - public boolean isCompleted() { - return fileUpload.isCompleted(); - } - - @Override - public boolean isInMemory() { - return fileUpload.isInMemory(); - } - - @Override - public long length() { - return fileUpload.length(); - } - - @Override - public long definedLength() { - return fileUpload.definedLength(); - } - - @Override - public boolean renameTo(File dest) throws IOException { - return fileUpload.renameTo(dest); - } - - @Override - public void setCharset(Charset charset) { - fileUpload.setCharset(charset); - } - - @Override - public void setContent(ByteBuf buffer) throws IOException { - try { - checkSize(buffer.readableBytes()); - } catch (IOException e) { - buffer.release(); - throw e; - } - if (buffer.readableBytes() > limitSize) { - if (fileUpload instanceof MemoryFileUpload) { - FileUpload memoryUpload = fileUpload; - // change to Disk - fileUpload = new DiskFileUpload(memoryUpload - .getName(), memoryUpload.getFilename(), memoryUpload - .getContentType(), memoryUpload - .getContentTransferEncoding(), memoryUpload.getCharset(), - definedSize, baseDir, deleteOnExit); - fileUpload.setMaxSize(maxSize); - - // release old upload - memoryUpload.release(); - } - } - fileUpload.setContent(buffer); - } - - @Override - public void setContent(File file) throws IOException { - checkSize(file.length()); - if (file.length() > limitSize) { - if (fileUpload instanceof MemoryFileUpload) { - FileUpload memoryUpload = fileUpload; - - // change to Disk - fileUpload = new DiskFileUpload(memoryUpload - .getName(), memoryUpload.getFilename(), memoryUpload - .getContentType(), memoryUpload - .getContentTransferEncoding(), memoryUpload.getCharset(), - definedSize, baseDir, deleteOnExit); - fileUpload.setMaxSize(maxSize); - - // release old upload - memoryUpload.release(); - } - } - fileUpload.setContent(file); - } - - @Override - public void setContent(InputStream inputStream) throws IOException { - if (fileUpload instanceof MemoryFileUpload) { - FileUpload memoryUpload = fileUpload; - - // change to Disk - fileUpload = new DiskFileUpload(fileUpload - .getName(), fileUpload.getFilename(), fileUpload - .getContentType(), fileUpload - .getContentTransferEncoding(), fileUpload.getCharset(), - definedSize, baseDir, deleteOnExit); - fileUpload.setMaxSize(maxSize); - - // release old upload - memoryUpload.release(); - } - fileUpload.setContent(inputStream); - } - - @Override - public void setContentType(String contentType) { - fileUpload.setContentType(contentType); + return wrapped.getFilename(); } @Override public void setContentTransferEncoding(String contentTransferEncoding) { - fileUpload.setContentTransferEncoding(contentTransferEncoding); + wrapped.setContentTransferEncoding(contentTransferEncoding); } @Override public void setFilename(String filename) { - fileUpload.setFilename(filename); - } - - @Override - public HttpDataType getHttpDataType() { - return fileUpload.getHttpDataType(); - } - - @Override - public String getName() { - return fileUpload.getName(); + wrapped.setFilename(filename); } @Override - public int hashCode() { - return fileUpload.hashCode(); - } - - @Override - public boolean equals(Object obj) { - return fileUpload.equals(obj); - } - - @Override - public int compareTo(InterfaceHttpData o) { - return fileUpload.compareTo(o); - } - - @Override - public String toString() { - return "Mixed: " + fileUpload; + public void setContentType(String contentType) { + wrapped.setContentType(contentType); } @Override - public ByteBuf getChunk(int length) throws IOException { - return fileUpload.getChunk(length); + public String getContentType() { + return wrapped.getContentType(); } @Override - public File getFile() throws IOException { - return fileUpload.getFile(); + FileUpload makeDiskData() { + DiskFileUpload diskFileUpload = new DiskFileUpload( + getName(), getFilename(), getContentType(), getContentTransferEncoding(), getCharset(), definedLength(), + baseDir, deleteOnExit); + diskFileUpload.setMaxSize(getMaxSize()); + return diskFileUpload; } @Override public FileUpload copy() { - return fileUpload.copy(); + // for binary compatibility + return super.copy(); } @Override public FileUpload duplicate() { - return fileUpload.duplicate(); + // for binary compatibility + return super.duplicate(); } @Override public FileUpload retainedDuplicate() { - return fileUpload.retainedDuplicate(); + // for binary compatibility + return super.retainedDuplicate(); } @Override public FileUpload replace(ByteBuf content) { - return fileUpload.replace(content); - } - - @Override - public ByteBuf content() { - return fileUpload.content(); - } - - @Override - public int refCnt() { - return fileUpload.refCnt(); - } - - @Override - public FileUpload retain() { - fileUpload.retain(); - return this; - } - - @Override - public FileUpload retain(int increment) { - fileUpload.retain(increment); - return this; + // for binary compatibility + return super.replace(content); } @Override public FileUpload touch() { - fileUpload.touch(); - return this; + // for binary compatibility + return super.touch(); } @Override public FileUpload touch(Object hint) { - fileUpload.touch(hint); - return this; + // for binary compatibility + return super.touch(hint); } @Override - public boolean release() { - return fileUpload.release(); + public FileUpload retain() { + // for binary compatibility + return super.retain(); } @Override - public boolean release(int decrement) { - return fileUpload.release(decrement); + public FileUpload retain(int increment) { + // for binary compatibility + return super.retain(increment); } } diff --git a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpDataTest.java b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpDataTest.java index 28a8dc0..2bd39e6 100644 --- a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpDataTest.java +++ b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpDataTest.java @@ -17,6 +17,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.PooledByteBufAllocator; +import io.netty.buffer.Unpooled; import io.netty.util.CharsetUtil; import org.assertj.core.api.ThrowableAssert; import org.junit.jupiter.api.BeforeAll; @@ -68,6 +69,20 @@ void testAddContentEmptyBuffer(HttpData httpData) throws IOException { assertThat(content.refCnt()).isEqualTo(0); } + @ParameterizedHttpDataTest + void testCompletedFlagPreservedAfterRetainDuplicate(HttpData httpData) throws IOException { + httpData.addContent(Unpooled.wrappedBuffer("foo".getBytes(CharsetUtil.UTF_8)), false); + assertThat(httpData.isCompleted()).isFalse(); + HttpData duplicate = httpData.retainedDuplicate(); + assertThat(duplicate.isCompleted()).isFalse(); + assertThat(duplicate.release()).isTrue(); + httpData.addContent(Unpooled.wrappedBuffer("bar".getBytes(CharsetUtil.UTF_8)), true); + assertThat(httpData.isCompleted()).isTrue(); + duplicate = httpData.retainedDuplicate(); + assertThat(duplicate.isCompleted()).isTrue(); + assertThat(duplicate.release()).isTrue(); + } + @Test void testAddContentExceedsDefinedSizeDiskFileUpload() { doTestAddContentExceedsSize( diff --git a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostRequestDecoderTest.java b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostRequestDecoderTest.java index 1abe171..b7dfb83 100644 --- a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostRequestDecoderTest.java +++ b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostRequestDecoderTest.java @@ -546,7 +546,7 @@ public void testMultipartRequestWithFieldInvalidCharset() throws Exception { @Test public void testFormEncodeIncorrect() throws Exception { LastHttpContent content = new DefaultLastHttpContent( - Unpooled.copiedBuffer("project=netty&&project=netty", CharsetUtil.US_ASCII)); + Unpooled.copiedBuffer("project=netty&=netty&project=netty", CharsetUtil.US_ASCII)); DefaultHttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/"); HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req); try { @@ -768,7 +768,7 @@ public void testMultipartRequest() throws Exception { @Test public void testNotLeak() { final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", - Unpooled.copiedBuffer("a=1&&b=2", CharsetUtil.US_ASCII)); + Unpooled.copiedBuffer("a=1&=2&b=3", CharsetUtil.US_ASCII)); try { assertThrows(HttpPostRequestDecoder.ErrorDataDecoderException.class, new Executable() { @Override diff --git a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoderTest.java b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoderTest.java new file mode 100644 index 0000000..d4f78df --- /dev/null +++ b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/HttpPostStandardRequestDecoderTest.java @@ -0,0 +1,90 @@ +/* + * Copyright 2022 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.contrib.handler.codec.http.multipart; + +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.util.CharsetUtil; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class HttpPostStandardRequestDecoderTest { + + @Test + void testDecodeAttributes() { + String requestBody = "key1=value1&key2=value2"; + + 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(2, decoder.getBodyHttpDatas().size()); + assertMemoryAttribute(decoder.getBodyHttpData("key1"), "value1"); + assertMemoryAttribute(decoder.getBodyHttpData("key2"), "value2"); + decoder.destroy(); + } + + @Test + void testDecodeAttributesWithAmpersandPrefixSkipsNullAttribute() { + String requestBody = "&key1=value1"; + + 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(1, decoder.getBodyHttpDatas().size()); + assertMemoryAttribute(decoder.getBodyHttpData("key1"), "value1"); + decoder.destroy(); + } + + @Test + void testDecodeZeroAttributesWithAmpersandPrefix() { + String requestBody = "&"; + + 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(); + } + + private static DefaultHttpDataFactory httpDiskDataFactory() { + return new DefaultHttpDataFactory(false); + } + + private static void assertMemoryAttribute(InterfaceHttpData data, String expectedValue) { + assertEquals(InterfaceHttpData.HttpDataType.Attribute, data.getHttpDataType()); + assertEquals(((MemoryAttribute) data).getValue(), expectedValue); + } + +} diff --git a/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/MixedTest.java b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/MixedTest.java new file mode 100644 index 0000000..d6540c9 --- /dev/null +++ b/codec-multipart/src/test/java/io/netty/contrib/handler/codec/http/multipart/MixedTest.java @@ -0,0 +1,57 @@ +/* + * Copyright 2022 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.contrib.handler.codec.http.multipart; + +import io.netty.buffer.Unpooled; +import io.netty.util.CharsetUtil; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +public class MixedTest { + @Test + public void mixedAttributeRefCnt() throws IOException { + MixedAttribute attribute = new MixedAttribute("foo", 100); + Assertions.assertEquals(1, attribute.refCnt()); + attribute.retain(); + Assertions.assertEquals(2, attribute.refCnt()); + + attribute.addContent(Unpooled.wrappedBuffer(new byte[90]), false); + Assertions.assertEquals(2, attribute.refCnt()); + + attribute.addContent(Unpooled.wrappedBuffer(new byte[90]), true); + Assertions.assertEquals(2, attribute.refCnt()); + + attribute.release(2); + } + + @Test + public void mixedFileUploadRefCnt() throws IOException { + MixedFileUpload upload = new MixedFileUpload("foo", "foo", "foo", "UTF-8", CharsetUtil.UTF_8, 0, 100); + Assertions.assertEquals(1, upload.refCnt()); + upload.retain(); + Assertions.assertEquals(2, upload.refCnt()); + + upload.addContent(Unpooled.wrappedBuffer(new byte[90]), false); + Assertions.assertEquals(2, upload.refCnt()); + + upload.addContent(Unpooled.wrappedBuffer(new byte[90]), true); + Assertions.assertEquals(2, upload.refCnt()); + + upload.release(2); + } +}