Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep completed flag for retained/duplicated HttpData #12576

Merged
merged 1 commit into from Jul 8, 2022

Conversation

pderop
Copy link
Contributor

@pderop pderop commented Jul 7, 2022

Motivation:

When you retain and duplicate a multipart HttpData, then the "isCompleted" flag is not preserved.

Example:
Using the following code:

        HttpData data = new MemoryAttribute("test", 0),
        data.addContent(Unpooled.wrappedBuffer("foo".getBytes(CharsetUtil.UTF_8)), false);
        HttpData duplicate = httpDatadata.retainedDuplicate();

then data.isCompleted() returns false, but duplicate.isCompleted() will return true

Modification:

retainDuplicate actually calls replace method, and when the various replace methods are calling setContent(), then after the call to setContent, the isCompleted flag will be reset to true.

The propose patch adds a new setCompleted(boolean) method in AbstractHttpData class, and then all the replace methods ensure that the setCompleted method is called after having called setContent.

Example for the replace method implemented by the MemoryFileUpload class:

    @Override
    public FileUpload replace(ByteBuf content) {
        MemoryFileUpload upload = new MemoryFileUpload(
                getName(), getFilename(), getContentType(), getContentTransferEncoding(), getCharset(), size);
        if (content != null) {
            try {
                upload.setContent(content);
            } catch (IOException e) {
                throw new ChannelException(e);
            }
        }
        upload.setCompleted(isCompleted());
        return upload;
    }

Result:

The isCompleted flag is preserved in the HttpData returned by the retainedDuplicate() method, so users can check if the HttpData is partial or not.

thanks.

@pderop
Copy link
Contributor Author

pderop commented Jul 7, 2022

@chrisvest , @normanmaurer , can you please take a look ? thanks !

@normanmaurer normanmaurer added this to the 4.1.79.Final milestone Jul 8, 2022
@normanmaurer normanmaurer merged commit c949fa6 into netty:4.1 Jul 8, 2022
@normanmaurer
Copy link
Member

Thanks a lot!

@pderop
Copy link
Contributor Author

pderop commented Jul 8, 2022

Thanks for the merge 👍 , I will now apply this fix to the netty-contrib/netty5 multipart porting PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants