eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.31k stars 2.08k forks source link

Multipart formdata filename special characters removed #4339

Open jaydev-ziroh opened 2 years ago

jaydev-ziroh commented 2 years ago

Version

4.2.7

Context

I have created a file upload server using vertx. the files are sent as multipart form data. the server should accept files with names containing special characters ( = specifically). vertx internally replaces special characters in the name of file to space character(byte representation 32)

Do you have a reproducer?

import io.vertx.core.Vertx;
import io.vertx.ext.web.FileUpload;
import io.vertx.ext.web.Router;
import io.vertx.ext.web.handler.BodyHandler;

import java.util.Set;

public class FileTest {

    public static void main(String[] args) {
        Vertx vertx = Vertx.vertx();
        Router router = Router.router(vertx);
        var bodyHandler = BodyHandler.create()
                .setDeleteUploadedFilesOnEnd(true)
                .setUploadsDirectory("<uploadDir>")
                .setHandleFileUploads(true);
        router.post("/files")
                .handler(bodyHandler)
                .handler(context->{
                    Set<FileUpload> fileUploads = context.fileUploads();
                    for (FileUpload fileUpload : fileUploads) {
                        System.out.println(fileUpload.fileName());
                    }
                    context.end();
                });
        vertx.createHttpServer().requestHandler(router).listen(8080);
    }
}

Steps to reproduce

  1. create the http server from above code.
  2. send a postman request as shown below ( the filename should be same as shown in the image
image

server side output will be ab cd.txt

Extra

openjdk 17 hotspot

I was also able to backtrack the issue and found out this code in io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.

protected InterfaceHttpData getFileUpload(String delimiter) {
        // eventually restart from existing FileUpload
        // Now get value according to Content-Type and Charset
        Attribute encoding = currentFieldAttributes.get(HttpHeaderNames.CONTENT_TRANSFER_ENCODING);
        Charset localCharset = charset;
        // Default
        TransferEncodingMechanism mechanism = TransferEncodingMechanism.BIT7;
        if (encoding != null) {
            String code;
            try {
                code = encoding.getValue().toLowerCase();
            } catch (IOException e) {
                throw new ErrorDataDecoderException(e);
            }
            if (code.equals(HttpPostBodyUtil.TransferEncodingMechanism.BIT7.value())) {
                localCharset = CharsetUtil.US_ASCII;
            } else if (code.equals(HttpPostBodyUtil.TransferEncodingMechanism.BIT8.value())) {
                localCharset = CharsetUtil.ISO_8859_1;
                mechanism = TransferEncodingMechanism.BIT8;
            } else if (code.equals(HttpPostBodyUtil.TransferEncodingMechanism.BINARY.value())) {
                // no real charset, so let the default
                mechanism = TransferEncodingMechanism.BINARY;
            } else {
                throw new ErrorDataDecoderException("TransferEncoding Unknown: " + code);
            }
        }
        Attribute charsetAttribute = currentFieldAttributes.get(HttpHeaderValues.CHARSET);
        if (charsetAttribute != null) {
            try {
                localCharset = Charset.forName(charsetAttribute.getValue());
            } catch (IOException e) {
                throw new ErrorDataDecoderException(e);
            } catch (UnsupportedCharsetException e) {
                throw new ErrorDataDecoderException(e);
            }
        }
        if (currentFileUpload == null) {
            Attribute filenameAttribute = currentFieldAttributes.get(HttpHeaderValues.FILENAME);
            Attribute nameAttribute = currentFieldAttributes.get(HttpHeaderValues.NAME);
            Attribute contentTypeAttribute = currentFieldAttributes.get(HttpHeaderNames.CONTENT_TYPE);
            Attribute lengthAttribute = currentFieldAttributes.get(HttpHeaderNames.CONTENT_LENGTH);
            long size;
            try {
                size = lengthAttribute != null ? Long.parseLong(lengthAttribute.getValue()) : 0L;
            } catch (IOException e) {
                throw new ErrorDataDecoderException(e);
            } catch (NumberFormatException ignored) {
                size = 0;
            }
            try {
                String contentType;
                if (contentTypeAttribute != null) {
                    contentType = contentTypeAttribute.getValue();
                } else {
                    contentType = HttpPostBodyUtil.DEFAULT_BINARY_CONTENT_TYPE;
                }
                currentFileUpload = factory.createFileUpload(request,
                        cleanString(nameAttribute.getValue()), cleanString(filenameAttribute.getValue()),
                        contentType, mechanism.value(), localCharset,
                        size);
            } catch (NullPointerException e) {
                throw new ErrorDataDecoderException(e);
            } catch (IllegalArgumentException e) {
                throw new ErrorDataDecoderException(e);
            } catch (IOException e) {
                throw new ErrorDataDecoderException(e);
            }
        }
        // load data as much as possible
        if (!loadDataMultipartOptimized(undecodedChunk, delimiter, currentFileUpload)) {
            // Delimiter is not found. Need more chunks.
            return null;
        }
        if (currentFileUpload.isCompleted()) {
            // ready to load the next one
            if (currentStatus == MultiPartStatus.FILEUPLOAD) {
                currentStatus = MultiPartStatus.HEADERDELIMITER;
                currentFieldAttributes = null;
            } else {
                currentStatus = MultiPartStatus.MIXEDDELIMITER;
                cleanMixedAttributes();
            }
            FileUpload fileUpload = currentFileUpload;
            currentFileUpload = null;
            return fileUpload;
        }
        // do not change the buffer position
        // since some can be already saved into FileUpload
        // So do not change the currentStatus
        return null;
    }
private static String cleanString(String field) {
        int size = field.length();
        StringBuilder sb = new StringBuilder(size);
        for (int i = 0; i < size; i++) {
            char nextChar = field.charAt(i);
            switch (nextChar) {
            case HttpConstants.COLON:
            case HttpConstants.COMMA:
            case HttpConstants.EQUALS:
            case HttpConstants.SEMICOLON:
            case HttpConstants.HT:
                sb.append(HttpConstants.SP_CHAR);
                break;
            case HttpConstants.DOUBLE_QUOTE:
                // nothing added, just removes it
                break;
            default:
                sb.append(nextChar);
                break;
            }
        }
        return sb.toString().trim();
    }

the cleanstring method is doing this when cleanString(filenameAttribute.getValue()) is called in code snippet 1

pmlopes commented 2 years ago

@jaydev-ziroh the translation happens as required by the HTTP RFC, I believe it comes from:

https://datatracker.ietf.org/doc/html/rfc2616#section-2.2

Where it states that a token cannot contain a separator, which includes =.

I think if you consider this a bug then it should be raised in netty as we are just consuming the object they provide and cannot change it.