[kinetic] Close sockets when server responds with HTTP/1.0 #1284
Conversation
mgrrx
force-pushed
the
fix-1283
branch
3 times, most recently
from
7ae55fd to
0de5a73
Compare
| return false; | ||
| } | ||
|
|
||
| _response = ""; | ||
|
|
||
| if (_header.size() < 8 || _header.find("HTTP/") != 0) | ||
| XmlRpcUtil::error("Error XmlRpcClient::parseResponse: Header does not start with protocol"); |
There was a problem hiding this comment.
In case a response is received which doesn't contain the header this patch will add a new error. Is this really necessary?
I am also not sure about if it is reasonable to change the current behavior for HTTP 1.0 connections to not-keep-them-open.
Also how does this related to #1287?
There was a problem hiding this comment.
It's not really necessary, I can remove it.
There was a problem hiding this comment.
@mgrrx This comment seems to be still pending.
There was a problem hiding this comment.
I simplified the state ment. I assume it does the job without introducing the new error message.
There was a problem hiding this comment.
I assume it does the job without introducing the new error message.
Please make sure to test the latest change since this is a significant change for a distro which is already stable for 4 years (and the other comment suggests it doesn't work correctly atm).
|
If #1287 gets merged should this PR be closed or still considered to be merged? |
|
Sorry for coming back to this after so many months. Let me summarize why I think this PR still makes sense, especially for older releases: |
|
@mgrrx Please consider retargeting a different branch. Lunar is already EOL. Either target kinetic-devel (if it should only be applied to Kinetic) or melodic-devel. |
|
Updated as requested |
dirk-thomas
changed the title
This PR tries to detect that the server responds with HTTP/1.0 and closes the connection accordingly in the manager.
I moved the clearing part of header_ to the end of parseResponse(). Putting setKeepOpen(false) into readHeader() didn't work for me as the internal state machine is affected by it. The header_ member is public but I couldn't find any usage of it, so I would assume that this doesn't affect anybody.