Merge pull request #16 from Azure/iisnode-dev

dpolivy - X-Forwarded-For improvements

README.md

@@ -46,7 +46,9 @@ This can be a head-scratcher since IIS Express 8 gives you both 32-bit and 64-bi
 - Install the full x64 version, then in Visual Studio go to Tools > Options > Projects and Solutions > Web Projects > Use the 64 bit version of IIS Express. This way you have a single install for both IIS and IIS Express.
 - Separately install iisnode express version (https://github.com/azure/iisnode/wiki/iisnode-releases).
 
+
 **Howtos**
+=======
 - [the basics](http://tomasz.janczuk.org/2011/08/hosting-nodejs-applications-in-iis-on.html)
 - [the basics (Pусский перевод)](http://softdroid.net/hosting-nodejs-applications-ru)
 - [**NEW: websockets**] (http://tomasz.janczuk.org/2012/11/how-to-use-websockets-with-nodejs-apps.html)

src/iisnode/chttpprotocol.cpp

@@ -168,6 +168,9 @@ HRESULT CHttpProtocol::SerializeRequestHeaders(CNodeHttpStoredContext* ctx, void
 		PSOCKADDR addr = request->GetRemoteAddress();
 		DWORD addrSize = addr->sa_family == AF_INET ? sizeof SOCKADDR_IN : sizeof SOCKADDR_IN6;
 		ErrorIf(0 != GetNameInfo(addr, addrSize, remoteHost, remoteHostSize, NULL, 0, NI_NUMERICHOST), GetLastError());
+
+		// Set remoteHostSize to the size of remoteHost
+		remoteHostSize = strlen(remoteHost);
 	}
 
 	for (int i = 0; i < raw->Headers.UnknownHeaderCount; i++)
@@ -178,10 +181,14 @@ HRESULT CHttpProtocol::SerializeRequestHeaders(CNodeHttpStoredContext* ctx, void
 
 		if (addXFF && 15 == raw->Headers.pUnknownHeaders[i].NameLength && 0 == _stricmp("X-Forwarded-For", raw->Headers.pUnknownHeaders[i].pName))
 		{
-			// augment existing X-Forwarded-For header
-
-			CheckError(CHttpProtocol::Append(context, ", ", 2, result, &bufferLength, &offset));
-			CheckError(CHttpProtocol::Append(context, remoteHost, 0, result, &bufferLength, &offset));
+			// augment existing X-Forwarded-For header, but only if the last item isn't the same IP as what we are adding
+			// (fixes https://github.com/tjanczuk/iisnode/issues/340)			 
+			if (raw->Headers.pUnknownHeaders[i].RawValueLength < remoteHostSize ||
+				0 != _stricmp(remoteHost, (raw->Headers.pUnknownHeaders[i].pRawValue + (raw->Headers.pUnknownHeaders[i].RawValueLength - remoteHostSize))))
+			{
+				CheckError(CHttpProtocol::Append(context, ", ", 2, result, &bufferLength, &offset));
+				CheckError(CHttpProtocol::Append(context, remoteHost, 0, result, &bufferLength, &offset));
+			}
 
 			addXFF = FALSE;
 		}

test/functional/tests/118_xff.js

@@ -6,5 +6,7 @@ var iisnodeassert = require("iisnodeassert");
 
 iisnodeassert.sequence([
     iisnodeassert.get(10000, "/118_xff/hello.js", 200, "Request contains X-Forwarded-For and X-Forwarded-Proto headers", { 'x-echo-x-forwarded-for': '127.0.0.1', 'x-echo-x-forwarded-proto': 'http' }),
-    iisnodeassert.post(10000, "/118_xff/hello.js", { headers: { 'X-Forwarded-Proto': 'https' }}, 200, "Request contains X-Forwarded-For and X-Forwarded-Proto headers", { 'x-echo-x-forwarded-for': '127.0.0.1', 'x-echo-x-forwarded-proto': 'https' })
+    iisnodeassert.post(10000, "/118_xff/hello.js", { headers: { 'X-Forwarded-Proto': 'https' }}, 200, "Request contains X-Forwarded-For and X-Forwarded-Proto headers", { 'x-echo-x-forwarded-for': '127.0.0.1', 'x-echo-x-forwarded-proto': 'https' }),
+    iisnodeassert.post(10000, "/118_xff/hello.js", { headers: { 'X-Forwarded-For': '1.1.1.1', 'X-Forwarded-Proto': 'https' }}, 200, "Request contains X-Forwarded-For and X-Forwarded-Proto headers", { 'x-echo-x-forwarded-for': '1.1.1.1, 127.0.0.1', 'x-echo-x-forwarded-proto': 'https' }),
+    iisnodeassert.post(10000, "/118_xff/hello.js", { headers: { 'X-Forwarded-For': '127.0.0.1', 'X-Forwarded-Proto': 'https' }}, 200, "Request contains X-Forwarded-For and X-Forwarded-Proto headers", { 'x-echo-x-forwarded-for': '127.0.0.1', 'x-echo-x-forwarded-proto': 'https' })
 ]);