Make libeay32.dll and ssleay32.dll visible to pluggable transports
When we moved the pluggable transport executables into a PluggableTransports subdirectory, it broke some pluggable transport programs on Windows because they can't find the OpenSSL DLLs. To wit, it broke flashproxy-reg-appspot and flashproxy-reg-email, because they use the M2Crypto Python module, which relies on OpenSSL. The effect was that the windows bundle was falling back to flashproxy-reg-http.
This patch makes a copy of libeay32.dll and ssleay32.dll in the PluggableTransports directory. This works because the directory containing an executable is part of the DLL search path of the executable. http://msdn.microsoft.com/en-us/library/7d83bc18.aspx
Nothing is required for linux and mac because their RelativeLink scripts set LD_LIBRARY_PATH and DYLD_LIBRARY_PATH respectively to contain the directory with the OpenSSL dynamic libraries.
Alternative solutions may be to modify the PATH environment variable to include the directory containing tor.exe (which would have an effect similar to that of setting LD_LIBRARY_PATH in RelativeLink.sh), or to call SetDllDirectory (I don't know if SetDllDirectory is inherited by subprocesses).
Activity
Here's a patch for RelativeLink.c that puts the Tor directory at the beginning of PATH. It has some groveling with C string concatenation and the Windows API. I'm not an expert at the Windows API but I checked the docs for the functions I used (GetCurrentDirectory, GetEnvironmentVariable).
Trac:
Status: new to needs_review
Parent: N/A to #9444 (closed)len is DWORD, it's unsigned. len should be int.
GetCurrentDirectory and GetEnvironmentVariable returns length of string not including nul-terminating character, but require output buffer with size including the null-terminating character. "n++" needs just after gets buffer size from those functions, else contents of buffer are undefined and no error return while fills buffer.Actually I can't get what values returned if NULL instead of buffer, it's unclear for me.Replying to cypherpunks:
len is DWORD, it's unsigned. len should be int.
You are right. I didn't know DWORD was unsigned. Compiling with
-Wextrashows:RelativeLink.c: In function ‘GetSubdirectory’: RelativeLink.c:54:5: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] RelativeLink.c: In function ‘PrependToPath’: RelativeLink.c:103:5: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]GetCurrentDirectory and GetEnvironmentVariable returns length of string not including nul-terminating character, but require output buffer with size including the null-terminating character.
I had to check this again because the interface is confusing. When these functions fail because the buffer is too small, they return the number of needed characters including the null terminator; i.e., the number of tchars you need to pass to malloc. When they succeed, they return the number of characters written not including the null terminator. GetCurrentDirectory: If the function succeeds, the return value specifies the number of characters that are written to the buffer, not including the terminating null character. If the function fails, the return value is zero. To get extended error information, call GetLastError. If the buffer that is pointed to by lpBuffer is not large enough, the return value specifies the required size of the buffer, in characters, including the null-terminating character. GetEnvironmentVariable: If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, not including the terminating null character. If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters, required to hold the string and its terminating null character and the contents of lpBuffer are undefined. If the function fails, the return value is zero. If the specified environment variable was not found in the environment block, GetLastError returns ERROR_ENVVAR_NOT_FOUND.
See http://msdn.microsoft.com/en-us/library/windows/desktop/ms682009%28v=vs.85%29.aspx#example_2, where they pass the return value of GetEnvironmentVariable directly to malloc.
I'd like to know whether the TCHAR stuff looks right. I used TEXT on string literals, multiplied malloc arguments by sizeof(TCHAR), and used _tcslen and _sntprintf in place of strlen and snprintf. I don't know whether the program is getting compiled in _UNICODE mode or not, or how to check that.
I attached a new patch that uses int for len, adds a missing error check to the second calls to GetCurrentDirectory and GetEnvironmentVariable, and expresses the size of some memory allocations in a way that indicates what it going to be stored in them.
Trac:
bug10845-apart.patchTrac:
bug10845-together.patchI'd like to know whether the TCHAR stuff looks right. I used TEXT on string literals, multiplied malloc arguments by sizeof(TCHAR), and used _tcslen and _sntprintf in place of strlen and snprintf. I don't know whether the program is getting compiled in _UNICODE mode or not, or how to check that.
I think it's right to do if planned to produce ANSI and UNICODE version of exe for RelativeLink.c. By default (if nothing defined) it builds as ANSI, and only side-effect is lack of support unicode-encoded paths if different code page.
If UNICODE version need then both UNICODE and _UNICODE should be defined before including windows specific headers (windows.h and tchar.h, as UNICODE for windows and _UNICODE for tchar). (At least for native mingw it fails to compile if only one defined.)
Replying to mikeperry:
We have had people complain in the past with issues when Unicode chars were used in path prefixes of the TBB. Do we want to build the unicode version of this for that reason?
Otherwise I think this looks good.
I did try it in a path with a Chinese directory name, and something else failed before it got to the point where the pluggable transport would have started. (I just went to zh.wikipedia.org, copied some text, and pasted it as a directory name, if you want to try it.) Building it in Unicode mode seems like the right idea, but there might be other issues elsewhere.
mentioned in issue tpo/applications/tor-browser#41570 (closed)
