You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "William R. Speirs (JIRA)" <ji...@apache.org> on 2012/07/19 00:13:34 UTC
[jira] [Created] (HTTPCORE-302) setEntity should consume the old
entity first
William R. Speirs created HTTPCORE-302:
------------------------------------------
Summary: setEntity should consume the old entity first
Key: HTTPCORE-302
URL: https://issues.apache.org/jira/browse/HTTPCORE-302
Project: HttpComponents HttpCore
Issue Type: Improvement
Components: HttpCore
Affects Versions: 4.2.1
Reporter: William R. Speirs
The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Updated] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Oleg Kalnichevski updated HTTPCORE-302:
---------------------------------------
Fix Version/s: 4.3
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Updated] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "William R. Speirs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
William R. Speirs updated HTTPCORE-302:
---------------------------------------
Attachment: HTTPCORE-302v2.diff
Updates discussed, and added a bit more documentation to other methods
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3-alpha1
>
> Attachments: HTTPCORE-302.diff, HTTPCORE-302v2.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "William R. Speirs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418382#comment-13418382 ]
William R. Speirs commented on HTTPCORE-302:
--------------------------------------------
I'm guessing changing BasicHttpResponse.setEntity() to call EntityUtils.consumeQuietly() would be out of the question? Could also extend HttpResponse to have a version of setEntity(boolean consumeOldEntity), and keep the behavior of setEntity() to NOT consume the entity.
I would be against adding another class (MessageUtils), unless there are other such methods you'd like to see added but don't have a good home for. I'd rather see it added to EntityUtils as updateEntity().
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "William R. Speirs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13422198#comment-13422198 ]
William R. Speirs commented on HTTPCORE-302:
--------------------------------------------
Yes, make sense. How does the patch look? Thanks!
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Resolved] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Oleg Kalnichevski resolved HTTPCORE-302.
----------------------------------------
Resolution: Fixed
Fix Version/s: (was: 4.3)
4.3-alpha1
Patch checked in. I changed javadocs for HttpResponse#setEntity slightly. Please review.
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3-alpha1
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "William R. Speirs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423067#comment-13423067 ]
William R. Speirs commented on HTTPCORE-302:
--------------------------------------------
I'm not sure why the link to updateEntity() was removed. If I read the comment on setEntity() and knew that I needed to consume the input, my next question would be "how?" Maybe just a @see EntityUtils link added?
I also just noticed a small issue with updateEntity(), there is no null check on response before calling response.getEntity(). Added the check is easy, but what should we do if it actually is null? Throw an IOException or add IllegalArgumentException to the method?
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3-alpha1
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418297#comment-13418297 ]
Oleg Kalnichevski commented on HTTPCORE-302:
--------------------------------------------
The problem with the solution 2b is that it would require breaking the current API as the #setEntity method would need to throw IOExceptions. I personally think EntityUtils (or new MessageUtils class) is a better place for such functionality. Ideally HttpMessage and their implementations should remain plain java objects without any processing logic.
MessageUtils.updateEntity(HttpResponse, HttpEntity) throws IOException;
What do you think?
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Updated] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "William R. Speirs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
William R. Speirs updated HTTPCORE-302:
---------------------------------------
Attachment: HTTPCORE-302.diff
Attached patch which adds EntityUtils.updateEntity() and adds a warning to HttpResponse.setEntity(). Do we want to take the added step of deprecating HttpResponse.setEntity() ?
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13424359#comment-13424359 ]
Oleg Kalnichevski commented on HTTPCORE-302:
--------------------------------------------
Patch committed.
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3-alpha1
>
> Attachments: HTTPCORE-302.diff, HTTPCORE-302v2.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13420148#comment-13420148 ]
Oleg Kalnichevski commented on HTTPCORE-302:
--------------------------------------------
Quietly discarding I/O exceptions would be a rather bad idea, at least without logging them. Please note, though, if some extra processing logic is needed in a particular application context one can easily replace the default HTTP message classes with custom ones by using a custom HttpResponseFactory / HttpRequestFactory implementations.
---
class MyBasicHttpResponse extends BasicHttpResponse {
public MyBasicHttpResponse(ProtocolVersion ver, int code, String reason) {
super(ver, code, reason);
}
public MyBasicHttpResponse(StatusLine statusLine) {
super(statusLine);
}
@Override
public void setEntity(HttpEntity entity) {
EntityUtils.consumeQuietly(getEntity());
super.setEntity(entity);
}
}
class MyHttpResponseFactory implements HttpResponseFactory {
public HttpResponse newHttpResponse(ProtocolVersion ver, int status, HttpContext context) {
return new MyBasicHttpResponse(ver, status,
EnglishReasonPhraseCatalog.INSTANCE.getReason(status, Locale.US));
}
public HttpResponse newHttpResponse(StatusLine statusline, HttpContext context) {
return new MyBasicHttpResponse(statusline);
}
};
---
EntityUtils#updateEntity() should also be just fine. Would you like to contribute a patch?
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13422085#comment-13422085 ]
Oleg Kalnichevski commented on HTTPCORE-302:
--------------------------------------------
@William: I really do not think there is anything conceptually wrong with HttpResponse#setEntity(). In the context of a proxy the protocol handler must take some additional precautions to ensure old content is fully consumed in case it gets rewritten by the proxy. But I would consider it a fairly specialized scenario. Besides, as I said in this case one would be better off using a custom HttpResponse implementation. Does this make any sense?
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] [Commented] (HTTPCORE-302) setEntity should consume the old
entity first
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423092#comment-13423092 ]
Oleg Kalnichevski commented on HTTPCORE-302:
--------------------------------------------
Ah. I think I did it because the new method was available in the 4.3 code line only. We can certainly add a reference to it in SVN trunk. IllegalArgumentException in case the response object being null should be reasonable, shouldn't it?
Oleg
> setEntity should consume the old entity first
> ---------------------------------------------
>
> Key: HTTPCORE-302
> URL: https://issues.apache.org/jira/browse/HTTPCORE-302
> Project: HttpComponents HttpCore
> Issue Type: Improvement
> Components: HttpCore
> Affects Versions: 4.2.1
> Reporter: William R. Speirs
> Fix For: 4.3-alpha1
>
> Attachments: HTTPCORE-302.diff
>
>
> The setEntity() method of HttpResponse makes it very easy to leak connections if used without consuming the old entity first. I'm proposing the following enhancements:
> 1) add a Javadoc note to the setEntity() method of HttpResponse that says something like: "Implementations might not consume an existing entity. Callers should consume any existing entity before calling this method."
> 2) either add a consumeAndSetEntity() method to BasicHttpResponse which consumes an existing entity before setting the reference, OR change the implementation of setEntity() to consume any existing entity first. (My vote is for the later, although it will change existing behavior, it shouldn't negatively impact any callers.)
> The reason for this improvement request is simply that I've been burned a few times by calling setEntity() without first consuming the existing entity, and I think we could make this more user friendly. I'm happy to submit a patch once we decide what should be done with #2.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org