summaryrefslogtreecommitdiff
path: root/redfish-core/lib/redfish_sessions.hpp
diff options
context:
space:
mode:
authorEd Tanous <ed@tanous.net>2020-08-27 21:45:42 +0300
committerEd Tanous <ed@tanous.net>2020-09-03 21:55:17 +0300
commitd206b437e3fb79a89205a9fbe748b017d312ddfa (patch)
tree87286ecb08d1306c1caca9130c628c29199cc932 /redfish-core/lib/redfish_sessions.hpp
parentf97ddba7015a553f13639233f7e5d3fb0152d0b2 (diff)
downloadbmcweb-d206b437e3fb79a89205a9fbe748b017d312ddfa.tar.xz
Revert http::Request::socket() callback
Details on why this revert is needed are here. https://lists.ozlabs.org/pipermail/openbmc/2020-August/022478.html Appu and Ravi still have not commented. It should be noted, this also causes a memory leak in http connection, where connections refuse to be freed, because of a bad usage of shared_from_this. This code wasn't very well thought through, and needs rearchitected to not break the unit testability of bmcweb, nor cause memory leaks. https://github.com/openbmc/bmcweb/blob/218bd4746130aac22366968c8c9a34a929e45a3d/http/http_connection.h#L351 Is the memory leak in question. Specifically, this reverts: The /attachment download in LogServices. This needs reimplemented properly, but is an OEM property, so it shouldn't be a big deal to revert, and shouldn't break our redfish compliance. The IpAddress property in SessionService. I have no idea why this was injected, and it's functionally incorrect. IpAddresses are not related to a session, and IP addresses can change over the course of a session, so this property is already broken as written. I suspect the author really wanted RedfishEvent type logging, but that was too complex, so they half implemented this. Redfish SSE properties. This needs to be reimplemented similar to the patchset here: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/13948 Where the ownership of the HTTP connection does not leave the http framework. As written, the SSE implementation causes ownership issues, as there's no clear delineation of the ownership between HttpConnection and the SSE framework. Tested: On current master, running this command: wget -O- --no-http-keep-alive --no-check-certificate https://{bmc hostname}:18080/redfish/v1 Which should download the service root, then immediately close and destroy the connection, prints: (2020-08-28 16:55:24) [DEBUG "routing.h":1258] Matched rule '/redfish/v1/' 2 / 4 (2020-08-28 16:55:24) [DEBUG "http_response.h":130] calling completion handler (2020-08-28 16:55:24) [DEBUG "http_response.h":133] completion handler was valid (2020-08-28 16:55:24) [INFO "http_connection.h":429] Response: 0x1e1ee28 /redfish/v1 200 keepalive=0 (2020-08-28 16:55:24) [DEBUG "timer_queue.h":48] timer add inside: 0x1d3d1a8 7 (2020-08-28 16:55:24) [DEBUG "http_connection.h":751] 0x1e1ee28 timer added: 0x1d3d1a8 7 (2020-08-28 16:55:24) [DEBUG "http_connection.h":655] 0x1e1ee28 doWrite (2020-08-28 16:55:24) [DEBUG "http_connection.h":663] 0x1e1ee28 async_write 1555 bytes (2020-08-28 16:55:24) [DEBUG "http_connection.h":697] 0x1e1ee28 timer cancelled: 0x1d3d1a8 7 (2020-08-28 16:55:24) [DEBUG "http_connection.h":676] 0x1e1ee28 from write(1) Then stops. Note, that the connection was not destroyed, and has leaked. Once this patchset is added, the connection closes and destroys properly, and doesn't leak, so it prints the above, but also prints. (2020-08-28 16:27:10) [DEBUG "http_connection.h":305] 0x1d15c90 Connection closed, total 1 Ran Redfish service validator. Saw one unrelated failure due to UUID, all other things pass. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I18686037bf58f20389d31facc0d77020274d38a1
Diffstat (limited to 'redfish-core/lib/redfish_sessions.hpp')
-rw-r--r--redfish-core/lib/redfish_sessions.hpp7
1 files changed, 0 insertions, 7 deletions
diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp
index 1d8e3c4fcc..fbbffcb22b 100644
--- a/redfish-core/lib/redfish_sessions.hpp
+++ b/redfish-core/lib/redfish_sessions.hpp
@@ -227,13 +227,6 @@ class SessionCollection : public Node
}
#endif
-#ifdef BMCWEB_ENABLE_SSL
- clientIp =
- req.socket().next_layer().remote_endpoint().address().to_string();
-#else
- clientIp = req.socket().remote_endpoint().address().to_string();
-#endif
-
// User is authenticated - create session
std::shared_ptr<persistent_data::UserSession> session =
persistent_data::SessionStore::getInstance().generateUserSession(