Skip to content

Commit a55a948

Browse files
allanrbovictorhora
authored andcommitted
IIS: Remove body prebuffering again. Unneeded due to no lock on modsecProcessRequest.
1 parent f93709b commit a55a948

File tree

1 file changed

+21
-84
lines changed

1 file changed

+21
-84
lines changed

iis/mymodule.cpp

Lines changed: 21 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@
3232

3333
#include "winsock2.h"
3434

35-
// Used to hold each chunk of request body that gets read before the full ModSec engine is invoked
36-
typedef struct preAllocBodyChunk {
37-
preAllocBodyChunk* next;
38-
size_t length;
39-
void* data;
40-
} preAllocBodyChunk;
4135

4236
class REQUEST_STORED_CONTEXT : public IHttpStoredContext
4337
{
@@ -89,8 +83,6 @@ class REQUEST_STORED_CONTEXT : public IHttpStoredContext
8983
char *m_pResponseBuffer;
9084
ULONGLONG m_pResponseLength;
9185
ULONGLONG m_pResponsePosition;
92-
93-
preAllocBodyChunk* requestBodyBufferHead;
9486
};
9587

9688

@@ -296,43 +288,6 @@ REQUEST_STORED_CONTEXT *RetrieveIISContext(request_rec *r)
296288
return NULL;
297289
}
298290

299-
HRESULT GetRequestBodyFromIIS(IHttpRequest* pRequest, preAllocBodyChunk** head)
300-
{
301-
HRESULT hr = S_OK;
302-
HTTP_REQUEST * pRawRequest = pRequest->GetRawHttpRequest();
303-
preAllocBodyChunk** cur = head;
304-
while (pRequest->GetRemainingEntityBytes() > 0)
305-
{
306-
// Allocate memory for the preAllocBodyChunk linked list structure, and also the actual body content
307-
// HUGE_STRING_LEN is hardcoded because this is also hardcoded in apache2_io.c's call to ap_get_brigade
308-
preAllocBodyChunk* chunk = (preAllocBodyChunk*)malloc(sizeof(preAllocBodyChunk) + HUGE_STRING_LEN);
309-
chunk->next = NULL;
310-
311-
// Pointer to rest of allocated memory, for convenience
312-
chunk->data = chunk + 1;
313-
314-
DWORD readcnt = 0;
315-
hr = pRequest->ReadEntityBody(chunk->data, HUGE_STRING_LEN, false, &readcnt, NULL);
316-
if (ERROR_HANDLE_EOF == (hr & 0x0000FFFF))
317-
{
318-
free(chunk);
319-
hr = S_OK;
320-
break;
321-
}
322-
chunk->length = readcnt;
323-
324-
// Append to linked list
325-
*cur = chunk;
326-
cur = &(chunk->next);
327-
328-
if (hr != S_OK)
329-
{
330-
break;
331-
}
332-
}
333-
334-
return hr;
335-
}
336291

337292
HRESULT CMyHttpModule::ReadFileChunk(HTTP_DATA_CHUNK *chunk, char *buf)
338293
{
@@ -800,24 +755,6 @@ CMyHttpModule::OnBeginRequest(
800755
goto Finished;
801756
}
802757

803-
// Get request body without holding lock, because some clients may be slow at sending
804-
LeaveCriticalSection(&m_csLock);
805-
preAllocBodyChunk* requestBodyBufferHead = NULL;
806-
hr = GetRequestBodyFromIIS(pRequest, &requestBodyBufferHead);
807-
if (hr != S_OK)
808-
{
809-
goto FinishedWithoutLock;
810-
}
811-
EnterCriticalSection(&m_csLock);
812-
813-
// Get the config again, in case it changed during the time we released the lock
814-
hr = MODSECURITY_STORED_CONTEXT::GetConfig(pHttpContext, &pConfig);
815-
if (FAILED(hr))
816-
{
817-
hr = S_OK;
818-
goto Finished;
819-
}
820-
821758
if(pConfig->m_Config == NULL)
822759
{
823760
char *path;
@@ -890,8 +827,6 @@ CMyHttpModule::OnBeginRequest(
890827
rsc->m_pRequestRec = r;
891828
rsc->m_pHttpContext = pHttpContext;
892829
rsc->m_pProvider = pProvider;
893-
rsc->requestBodyBufferHead = requestBodyBufferHead;
894-
requestBodyBufferHead = NULL; // This is to indicate to the cleanup process to use rsc->requestBodyBufferHead instead of requestBodyBufferHead now
895830

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

@@ -1139,44 +1074,46 @@ CMyHttpModule::OnBeginRequest(
11391074
Finished:
11401075
LeaveCriticalSection(&m_csLock);
11411076

1142-
FinishedWithoutLock:
1143-
// Free the preallocated body in case there was a failure and it wasn't consumed already
1144-
preAllocBodyChunk* chunkToFree = requestBodyBufferHead ? requestBodyBufferHead : rsc->requestBodyBufferHead;
1145-
while (chunkToFree != NULL)
1146-
{
1147-
preAllocBodyChunk* next = chunkToFree->next;
1148-
free(chunkToFree);
1149-
chunkToFree = next;
1150-
}
1151-
11521077
if ( FAILED( hr ) )
11531078
{
11541079
return RQ_NOTIFICATION_FINISH_REQUEST;
11551080
}
11561081
return RQ_NOTIFICATION_CONTINUE;
11571082
}
11581083

1084+
11591085
apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos)
11601086
{
11611087
REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r);
11621088

1163-
if (rsc->requestBodyBufferHead == NULL)
1089+
*readcnt = 0;
1090+
1091+
if (rsc == NULL)
11641092
{
11651093
*is_eos = 1;
11661094
return APR_SUCCESS;
11671095
}
11681096

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

1173-
// Remove the front and proceed to next chunk in the linked list
1174-
preAllocBodyChunk* chunkToFree = rsc->requestBodyBufferHead;
1175-
rsc->requestBodyBufferHead = rsc->requestBodyBufferHead->next;
1176-
free(chunkToFree);
1100+
if (pRequest->GetRemainingEntityBytes() == 0)
1101+
{
1102+
*is_eos = 1;
1103+
return APR_SUCCESS;
1104+
}
1105+
1106+
HRESULT hr = pRequest->ReadEntityBody(buf, length, false, (DWORD *)readcnt, NULL);
11771107

1178-
if (rsc->requestBodyBufferHead == NULL)
1108+
if (FAILED(hr))
11791109
{
1110+
// End of data is okay.
1111+
if (ERROR_HANDLE_EOF != (hr & 0x0000FFFF))
1112+
{
1113+
// Set the error status.
1114+
rsc->m_pProvider->SetErrorStatus(hr);
1115+
}
1116+
11801117
*is_eos = 1;
11811118
}
11821119

0 commit comments

Comments
 (0)