From 64026880841fbcbababf531740046ffedb60f956 Mon Sep 17 00:00:00 2001 From: KLuka Date: Tue, 26 May 2015 22:36:56 +0200 Subject: [PATCH] Improved code session and URL handling * URL dependency was removed from session handling code. URL object was only needed to get session key from client request. This was moved somewhere else to achive better code reusability. * Added URL parsing functionality that can be used without URL object. --- libhttp/url.c | 14 ++++++++++---- libhttp/url.h | 2 ++ shellinabox/session.c | 31 ++++++++++--------------------- shellinabox/session.h | 10 ++++------ shellinabox/shellinaboxd.c | 19 +++++++++++-------- 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/libhttp/url.c b/libhttp/url.c index dd410bb..3cc7890 100644 --- a/libhttp/url.c +++ b/libhttp/url.c @@ -113,7 +113,7 @@ static char *urlMakeString(const char *buf, int len) { } } -static void urlParseQueryString(struct URL *url, const char *query, int len) { +static void urlParseQueryString(struct HashMap *hashmap, const char *query, int len) { const char *key = query; const char *value = NULL; for (const char *ampersand = query; len-- >= 0; ampersand++) { @@ -131,7 +131,7 @@ static void urlParseQueryString(struct URL *url, const char *query, int len) { v = urlMakeString(value, vl); urlUnescape(v); } - addToHashMap(&url->args, k, v); + addToHashMap(hashmap, k, v); } key = ampersand + 1; value = NULL; @@ -275,7 +275,7 @@ static void urlParsePostBody(struct URL *url, const char *ctHeader = getFromHashMap(&http->header, "content-type"); urlParseHeaderLine(&contentType, ctHeader, ctHeader ? strlen(ctHeader) : 0); if (getRefFromHashMap(&contentType, "application/x-www-form-urlencoded")) { - urlParseQueryString(url, buf, len); + urlParseQueryString(&url->args, buf, len); } else if (getRefFromHashMap(&contentType, "multipart/form-data")) { const char *boundary = getFromHashMap(&contentType, "boundary"); if (boundary && *boundary) { @@ -346,7 +346,7 @@ void initURL(struct URL *url, const struct HttpConnection *http, url->url = NULL; initHashMap(&url->args, urlDestroyHashMapEntry, NULL); if (!strcmp(http->method, "GET")) { - urlParseQueryString(url, url->query, strlen(url->query)); + urlParseQueryString(&url->args, url->query, strlen(url->query)); } else if (!strcmp(http->method, "POST")) { urlParsePostBody(url, http, buf, len); } @@ -428,3 +428,9 @@ const char *urlGetURL(struct URL *url) { const struct HashMap *urlGetArgs(struct URL *url) { return &url->args; } + +struct HashMap *urlParseQuery(const char *buf, int len) { + struct HashMap *hashmap = newHashMap(urlDestroyHashMapEntry, NULL); + urlParseQueryString(hashmap, buf, len); + return hashmap; +} diff --git a/libhttp/url.h b/libhttp/url.h index 7af8367..7931a5b 100644 --- a/libhttp/url.h +++ b/libhttp/url.h @@ -83,4 +83,6 @@ const char *urlGetAnchor(struct URL *url); const char *urlGetURL(struct URL *url); const struct HashMap *urlGetArgs(struct URL *url); +struct HashMap *urlParseQuery(const char *buf, int len); + #endif /* URL_H__ */ diff --git a/shellinabox/session.c b/shellinabox/session.c index 25f5513..cd22b3b 100644 --- a/shellinabox/session.c +++ b/shellinabox/session.c @@ -108,13 +108,12 @@ void checkGraveyard(void) { } void initSession(struct Session *session, const char *sessionKey, - Server *server, URL *url, const char *peerName) { + Server *server, const char *peerName) { session->sessionKey = sessionKey; session->server = server; check(session->peerName = strdup(peerName)); session->connection = NULL; session->http = NULL; - session->url = url; session->done = 0; session->pty = -1; session->width = 0; @@ -125,11 +124,11 @@ void initSession(struct Session *session, const char *sessionKey, session->cleanup = 0; } -struct Session *newSession(const char *sessionKey, Server *server, URL *url, +struct Session *newSession(const char *sessionKey, Server *server, const char *peerName) { struct Session *session; check(session = malloc(sizeof(struct Session))); - initSession(session, sessionKey, server, url, peerName); + initSession(session, sessionKey, server, peerName); return session; } @@ -137,7 +136,6 @@ void destroySession(struct Session *session) { if (session) { free((char *)session->peerName); free((char *)session->sessionKey); - deleteURL(session->url); if (session->pty >= 0) { NOINTR(close(session->pty)); } @@ -205,38 +203,33 @@ char *newSessionKey(void) { return sessionKey; } -struct Session *findCGISession(int *isNew, HttpConnection *http, URL *url, - const char *cgiSessionKey) { - *isNew = 1; +struct Session *findSession(const char *sessionKey, const char *cgiSessionKey, + int *sessionIsNew, HttpConnection *http) { + *sessionIsNew = 1; if (!sessions) { sessions = newHashMap(destroySessionHashEntry, NULL); } - const HashMap *args = urlGetArgs(url); - const char *sessionKey = getFromHashMap(args, "session"); + struct Session *session= NULL; if (cgiSessionKey && (!sessionKey || strcmp(cgiSessionKey, sessionKey))) { // In CGI mode, we only ever allow exactly one session with a // pre-negotiated key. - deleteURL(url); } else { if (sessionKey && *sessionKey) { session = (struct Session *)getFromHashMap(sessions, sessionKey); } if (session) { - *isNew = 0; - deleteURL(session->url); - session->url = url; + *sessionIsNew = 0; } else if (!cgiSessionKey && sessionKey && *sessionKey) { - *isNew = 0; + *sessionIsNew = 0; debug("Failed to find session: %s", sessionKey); - deleteURL(url); } else { // First contact. Create session, now. check(sessionKey = cgiSessionKey ? strdup(cgiSessionKey) : newSessionKey()); - session = newSession(sessionKey, httpGetServer(http), url, + session = newSession(sessionKey, httpGetServer(http), httpGetPeerName(http)); addToHashMap(sessions, sessionKey, (const char *)session); debug("Creating a new session: %s", sessionKey); @@ -245,10 +238,6 @@ struct Session *findCGISession(int *isNew, HttpConnection *http, URL *url, return session; } -struct Session *findSession(int *isNew, HttpConnection *http, URL *url) { - return findCGISession(isNew, http, url, NULL); -} - void iterateOverSessions(int (*fnc)(void *, const char *, char **), void *arg){ iterateOverHashMap(sessions, fnc, arg); } diff --git a/shellinabox/session.h b/shellinabox/session.h index 11d2281..73d0eb3 100644 --- a/shellinabox/session.h +++ b/shellinabox/session.h @@ -56,7 +56,6 @@ struct Session { ServerConnection *connection; const char *peerName; HttpConnection *http; - URL *url; int done; int pty; int width; @@ -70,8 +69,8 @@ struct Session { void addToGraveyard(struct Session *session); void checkGraveyard(void); void initSession(struct Session *session, const char *sessionKey, - Server *server, URL *url, const char *peerName); -struct Session *newSession(const char *sessionKey, Server *server, URL *url, + Server *server, const char *peerName); +struct Session *newSession(const char *sessionKey, Server *server, const char *peerName); void destroySession(struct Session *session); void deleteSession(struct Session *session); @@ -79,9 +78,8 @@ void abandonSession(struct Session *session); char *newSessionKey(void); void finishSession(struct Session *session); void finishAllSessions(void); -struct Session *findCGISession(int *isNew, HttpConnection *http, URL *url, - const char *cgiSessionKey); -struct Session *findSession(int *isNew, HttpConnection *http, URL *url); +struct Session *findSession(const char *sessionKey, const char *cgiSessionKey, + int *sessionIsNew, HttpConnection *http); void iterateOverSessions(int (*fnc)(void *, const char *, char **), void *arg); int numSessions(void); diff --git a/shellinabox/shellinaboxd.c b/shellinabox/shellinaboxd.c index 0df94bf..cb0014d 100644 --- a/shellinabox/shellinaboxd.c +++ b/shellinabox/shellinaboxd.c @@ -345,28 +345,29 @@ static int dataHandler(HttpConnection *http, struct Service *service, if (!buf) { // Somebody unexpectedly closed our http connection (e.g. because of a // timeout). This is the last notification that we will get. - deleteURL(url); iterateOverSessions(invalidatePendingHttpSession, http); return HTTP_DONE; } // Find an existing session, or create the record for a new one - int isNew; - struct Session *session = findCGISession(&isNew, http, url, cgiSessionKey); + const HashMap *args = urlGetArgs(url); + const char *sessionKey = getFromHashMap(args, "session"); + + int sessionIsNew; + struct Session *session = findSession(sessionKey, cgiSessionKey, &sessionIsNew, http); if (session == NULL) { httpSendReply(http, 400, "Bad Request", NO_MSG); return HTTP_DONE; } // Sanity check - if (!isNew && strcmp(session->peerName, httpGetPeerName(http))) { + if (!sessionIsNew && strcmp(session->peerName, httpGetPeerName(http))) { error("Peername changed from %s to %s", session->peerName, httpGetPeerName(http)); httpSendReply(http, 400, "Bad Request", NO_MSG); return HTTP_DONE; } - const HashMap *args = urlGetArgs(session->url); int oldWidth = session->width; int oldHeight = session->height; const char *width = getFromHashMap(args, "width"); @@ -381,7 +382,7 @@ static int dataHandler(HttpConnection *http, struct Service *service, } // Create a new session, if the client did not provide an existing one - if (isNew) { + if (sessionIsNew) { if (keys) { bad_new_session: abandonSession(session); @@ -456,7 +457,7 @@ static int dataHandler(HttpConnection *http, struct Service *service, session->connection = serverGetConnection(session->server, session->connection, session->pty); - if (session->buffered || isNew) { + if (session->buffered || sessionIsNew) { if (completePendingRequest(session, "", 0, MAX_RESPONSE) && session->connection) { // Reset the timeout, as we just received a new request. @@ -636,7 +637,9 @@ static int shellInABoxHttpHandler(HttpConnection *http, void *arg, !strncasecmp(contentType, "application/x-www-form-urlencoded", 33)) { // XMLHttpRequest carrying data between the AJAX application and the // client session. - return dataHandler(http, arg, buf, len, url); + int status = dataHandler(http, arg, buf, len, url); + deleteURL(url); + return status; } UNUSED(rootPageSize); char *html = stringPrintf(NULL, rootPageStart,