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

extended condition not to add the content-length header negative #4967

Merged

Conversation

kjezek
Copy link
Contributor

@kjezek kjezek commented Jan 17, 2022

This update prevents wrong adding of the Content-Length header with the value -1. Such a value is not defined in the specification. The header should not be present at all when the length is not defined.

@kjezek
Copy link
Contributor Author

kjezek commented Jan 17, 2022

This should fix: #4965

@jansupol
Copy link
Contributor

Thank you for the PR. The continuous integration check did fail because the copyright year is not updated to 2022.

The PR does not seem right. Why did you exclude zero-length content? Can you provide a test that demonstrates the use-case this PR helps?

@kjezek
Copy link
Contributor Author

kjezek commented Jan 17, 2022

Hi @jansupol

  1. I have fixed the license year issues.

  2. The Content-Length header must not be added to the header list when the length is negative. First, it is against the specification - when the length is not know, the header must be omitted, not used with a negative number. Secondly, the code I have fixed did originally add the wrong Content-Length header when the header was actually not present in the original response - i.e. it modified the response wrongly, adding a wrong header.

@jansupol
Copy link
Contributor

I can understand the negative number.
My question was why did you want to remove header for the zero length content, i.e. when entity.getContentLength() == 0?

@kjezek
Copy link
Contributor Author

kjezek commented Jan 17, 2022

@jansupol you are right, the zero length of Content-Length is a valid value. I have fixed the condition.

@senivam senivam merged commit 4106350 into eclipse-ee4j:master Jan 18, 2022
@senivam senivam added this to the 2.36 milestone Jan 18, 2022
This was referenced Jun 14, 2022
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.

4 participants