Skip to content

IIS: Remove body prebuffering again. Unneeded due to no lock on modse… #1917

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

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 21 additions & 84 deletions iis/mymodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@

#include "winsock2.h"

// Used to hold each chunk of request body that gets read before the full ModSec engine is invoked
typedef struct preAllocBodyChunk {
preAllocBodyChunk* next;
size_t length;
void* data;
} preAllocBodyChunk;

class REQUEST_STORED_CONTEXT : public IHttpStoredContext
{
Expand Down Expand Up @@ -89,8 +83,6 @@ class REQUEST_STORED_CONTEXT : public IHttpStoredContext
char *m_pResponseBuffer;
ULONGLONG m_pResponseLength;
ULONGLONG m_pResponsePosition;

preAllocBodyChunk* requestBodyBufferHead;
};


Expand Down Expand Up @@ -296,43 +288,6 @@ REQUEST_STORED_CONTEXT *RetrieveIISContext(request_rec *r)
return NULL;
}

HRESULT GetRequestBodyFromIIS(IHttpRequest* pRequest, preAllocBodyChunk** head)
{
HRESULT hr = S_OK;
HTTP_REQUEST * pRawRequest = pRequest->GetRawHttpRequest();
preAllocBodyChunk** cur = head;
while (pRequest->GetRemainingEntityBytes() > 0)
{
// Allocate memory for the preAllocBodyChunk linked list structure, and also the actual body content
// HUGE_STRING_LEN is hardcoded because this is also hardcoded in apache2_io.c's call to ap_get_brigade
preAllocBodyChunk* chunk = (preAllocBodyChunk*)malloc(sizeof(preAllocBodyChunk) + HUGE_STRING_LEN);
chunk->next = NULL;

// Pointer to rest of allocated memory, for convenience
chunk->data = chunk + 1;

DWORD readcnt = 0;
hr = pRequest->ReadEntityBody(chunk->data, HUGE_STRING_LEN, false, &readcnt, NULL);
if (ERROR_HANDLE_EOF == (hr & 0x0000FFFF))
{
free(chunk);
hr = S_OK;
break;
}
chunk->length = readcnt;

// Append to linked list
*cur = chunk;
cur = &(chunk->next);

if (hr != S_OK)
{
break;
}
}

return hr;
}

HRESULT CMyHttpModule::ReadFileChunk(HTTP_DATA_CHUNK *chunk, char *buf)
{
Expand Down Expand Up @@ -800,24 +755,6 @@ CMyHttpModule::OnBeginRequest(
goto Finished;
}

// Get request body without holding lock, because some clients may be slow at sending
LeaveCriticalSection(&m_csLock);
preAllocBodyChunk* requestBodyBufferHead = NULL;
hr = GetRequestBodyFromIIS(pRequest, &requestBodyBufferHead);
if (hr != S_OK)
{
goto FinishedWithoutLock;
}
EnterCriticalSection(&m_csLock);

// Get the config again, in case it changed during the time we released the lock
hr = MODSECURITY_STORED_CONTEXT::GetConfig(pHttpContext, &pConfig);
if (FAILED(hr))
{
hr = S_OK;
goto Finished;
}

if(pConfig->m_Config == NULL)
{
char *path;
Expand Down Expand Up @@ -890,8 +827,6 @@ CMyHttpModule::OnBeginRequest(
rsc->m_pRequestRec = r;
rsc->m_pHttpContext = pHttpContext;
rsc->m_pProvider = pProvider;
rsc->requestBodyBufferHead = requestBodyBufferHead;
requestBodyBufferHead = NULL; // This is to indicate to the cleanup process to use rsc->requestBodyBufferHead instead of requestBodyBufferHead now

pHttpContext->GetModuleContextContainer()->SetModuleContext(rsc, g_pModuleContext);

Expand Down Expand Up @@ -1139,44 +1074,46 @@ CMyHttpModule::OnBeginRequest(
Finished:
LeaveCriticalSection(&m_csLock);

FinishedWithoutLock:
// Free the preallocated body in case there was a failure and it wasn't consumed already
preAllocBodyChunk* chunkToFree = requestBodyBufferHead ? requestBodyBufferHead : rsc->requestBodyBufferHead;
while (chunkToFree != NULL)
{
preAllocBodyChunk* next = chunkToFree->next;
free(chunkToFree);
chunkToFree = next;
}

if ( FAILED( hr ) )
{
return RQ_NOTIFICATION_FINISH_REQUEST;
}
return RQ_NOTIFICATION_CONTINUE;
}


apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos)
{
REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r);

if (rsc->requestBodyBufferHead == NULL)
*readcnt = 0;

if (rsc == NULL)
{
*is_eos = 1;
return APR_SUCCESS;
}

*readcnt = length < (unsigned int) rsc->requestBodyBufferHead->length ? length : (unsigned int) rsc->requestBodyBufferHead->length;
void* src = (char*)rsc->requestBodyBufferHead->data;
memcpy_s(buf, length, src, *readcnt);
IHttpContext *pHttpContext = rsc->m_pHttpContext;
IHttpRequest *pRequest = pHttpContext->GetRequest();

// Remove the front and proceed to next chunk in the linked list
preAllocBodyChunk* chunkToFree = rsc->requestBodyBufferHead;
rsc->requestBodyBufferHead = rsc->requestBodyBufferHead->next;
free(chunkToFree);
if (pRequest->GetRemainingEntityBytes() == 0)
{
*is_eos = 1;
return APR_SUCCESS;
}

HRESULT hr = pRequest->ReadEntityBody(buf, length, false, (DWORD *)readcnt, NULL);

if (rsc->requestBodyBufferHead == NULL)
if (FAILED(hr))
{
// End of data is okay.
if (ERROR_HANDLE_EOF != (hr & 0x0000FFFF))
{
// Set the error status.
rsc->m_pProvider->SetErrorStatus(hr);
}

*is_eos = 1;
}

Expand Down