Bugzilla – Full Text Bug Listing
| Summary: | [Vista] HTTP Recorder requires work-around on Windows Vista for Internet Explorer | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | DuWayne Morris <dmorris> |
| Component: | TPTP | Assignee: | DuWayne Morris <dmorris> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P1 | CC: | jptoomey, kdsiefke, mddunn, paulslau, sluiman, toddmm |
| Version: | unspecified | Keywords: | plan |
| Target Milestone: | --- | Flags: | jptoomey:
review+
paulslau: review+ |
| Hardware: | PC | ||
| OS: | Windows Vista | ||
| Whiteboard: | |||
| Bug Depends on: | 167842, 171693, 171694, 177396, 179871 | ||
| Bug Blocks: | 163254 | ||
| Attachments: | |||
|
Description
DuWayne Morris
Changed severity to major. [Duwayne Morris] Fix has been outlined in the defect notes. A more elaborate solution should be targeted for 4.4. Make this defect dependent upon platform defect 167842 per Paul's comment. Please defer to 4.4 since the work-around will be documented for 4.2.2/4.3.1. I have tried to defer this to 4.4 but I am unable to modify the Target Milestone. Add estimate to include integration testing. As discussed on today's Test Project call, Duwayne will create a new defect and fix in i2 (e.g. EOW) for action 3 in the defect's description. This defect will remain in the system for resolving the root issue of changing the system registry. 4.4 is not a valid target If this is planned to be done in 4.4 it should be assigned to an iteration, if not please reset target to future Reset target to future. Mark, we cannot defer in-plan defects until we make a request to the PMC. Mark/Dwayne: Can you verify using the TPTP-4.4.0-200705040100 driver if this issue still exists given all of the dependency defects (and https://bugs.eclipse.org/bugs/show_bug.cgi?id=179871) have been resolved? (In reply to comment #7) > As discussed on today's Test Project call, Duwayne will create a new defect and > fix in i2 (e.g. EOW) for action 3 in the defect's description. > > This defect will remain in the system for resolving the root issue of changing > the system registry. > Duwayne, did the new defect get created/fixed? Bugzilla 177396 was created and subsequently fixed by Mark on March 15. Can you update the target and close the defect if the problem has been already fixed Comment #12 was to document that a separate defect was filed and fixed to at least warn the user to "Run as Administrator" when recording fails on Vista due to inability to launch regedt32.exe. It does not fix this defect. Buzilla 179871 referenced in comment #10 and/or any other fixes that may have been committed prior to the 05-04 TPTP build did not seem to address or fix this issue (verified by testing the latest build). As far as Mark and I know, nobody has proposed an acceptable solution path for this defect. If anyone monitoring this defect knows of an acceptable solution, please speak up. (In reply to comment #13) > Can you update the target and close the defect if the problem has been already > fixed > This defect has not yet been resolved (see comment 14) and is a candidate for deferral from 4.4. However, we have completed the release note and informational dialog to work-around the limitation. This defect also has been identified as a Vista issue not fixed in the 4.4 timeframe https://bugs.eclipse.org/bugs/show_bug.cgi?id=168597 This defect is a candidate for deferral from the 4.4 release. If the originator has opposition, please provide an argument against this deferral. can you assign this defect to a valid iteration Assigning to i4 but this defect belongs to a block of in-plan 4.4 defects that are candidates for deferral. Assigning to i4 since i3 is complete but this defect belongs to a block of in-plan 4.4 defects that are candidates for deferral. Amending summary to indicate that the problem is specific to IE (no workaround required for Firefox.) Adding new time estimate based on a proposed solution: The problem is caused by our failed attempt to edit IE's proxy configuration information in the registry (since we are not running as administrator, this is prohibited on Vista.) One proposed solution would be to move the registry editing code into the recorder, which is launched by the agent controller and inherits its permissions. There are still some concerns with this proposal: 1) If we are running with the IAC, this will still not work, since the IAC is not run as administrator. 2) We currently detect a number of possible failures in configuring the proxy via the registry and have translated strings for those conditions. We would need to add a mechanism to either pass the error strings back, or return an error ID that is looked up on the workbench side. Approved for deferral by PMC. Retargeting to 4.4.1 per PMC discussion. Please ensure that a fix for defect is being investigated, or raise the issue at the project level to find the necessary resources to address. Transferring to Duwayne. As discussed on today's Test Project call, the issue is that HTTP proxy recording using Internet Explorer (IE) requires regedit to update/restore the IE proxy settings. When a user starts a recording, the regedit process is executed from Eclipse. However, Vista required the regedit to be executed with 'run as administrator' privileges (http://www.eclipse.org/tptp/home/downloads/releasenotes/releasenotes4_4_0.html#4_CTI_6). The work-around has been to start Eclipse with 'run as administrator' privileges (e.g. <right-click Eclipse.exe> >> Run as administrator), even if logged in as administrator. One possible solution is to move the proxy configuration code to the agent side to acquire the Agent Controller's privileges but this solution would not resolve the IAC use case. The recommendation from Vista to only require 'run as administrator' privileges for select function and application-wide privileges. As such, the proposed solution when HTTP proxy recording using Internet Explorer (IE) on Vista ONLY includes when the execution of regedit fails (insufficient privileges), launch an executable with the 'run as administrator' privileges bit set to launch regedit. This will prompt the user for the administrator password and display a dialog to continue. This solution requires an additional native application (e.g. *.exe) to be created, built, and shipped with the HTTP Recorder. The next step is to create a proof of concept. For those on the CC list, please provide comments on this proposed solution. The bottom line solution that is being implemented is in the last paragraph of this comment.
The biggest reason for outlining the problems I have cited below is to provide some insight into how Vista interferes with writing code that will work. I personally have issues with the documentation that lacks any Vista support wording. For a simple example, if the API documents were to say that a non-elevated privilege application cannot succeed in calling RegSaveKey, it would save wasted time and the application programmer would be aware of the limitations. Granted, many of the API failures would likely be similar on XP when running with a normal user account instead of a user in the administrator group. However, other failures that seem to be totally unique to this new OS security environment.
The first approach that had been proposed and accepted was to use a separate application with elevated privileges as stated in the previous comment.
Windows Vista turned out to be much more difficult to deal with than planned. Working with Visual Studio 2005, a standalone executable was created with a linked in manifest that marks the application to require elevated privileges. Each time the executable was launched, Vista would prompt the user to allow the program to run. Within that application, regedit.exe could be launched without further prompting from Vista, as expected. The manifest to mark the application was as follows:
<?xml version="1.0" encoding="utf-8" ?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
<assemblyIdentity version="1.0.0.0"
processorArchitecture="X86"
name="TPTPRegistrySettingHandler.exe"
type="win32" />
<description>who cares</description>
<trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
<security>
<requestedPrivileges>
<requestedExecutionLevel level="requireAdministrator" />
</requestedPrivileges>
</security>
</trustInfo>
</assembly>
Then, IPC was needed to communicate with the elevated privilege application from the Workbench process space. One idea was to minimize changes in the underlying code and simply use a Win32 DLL and a named pipe to communicate with the elevated privilege process to run regedit.exe.
It turned out that Vista is concerned about having regular "non-elevated" processes controlling a process with elevated privileges. As a result, I found documents that stated, for example, that calls to SendMessage and PostMessage would succeed, but that Vista would not really deliver the messages to the elevated privilege application.
All attempts to use a named pipe on Vista failed to work in the manner needed. A pipe can be created to receive messages from a privileged application. However, a regular non-elevated application cannot succeed in opening the client end of the pipe calling CreateFile with GENERIC_WRITE permissions. Attempts to do so result in ERROR_ACCESS_DENIED. Thus, there was no way to send a message to the application. There were many attempts to make this work and discussion with others in Toronto on this subject.
I did not try socket communication, since using a socket was not viewed as satisfactory to all concerned team members.
As a result of all that background, I decided to proceed with just a native DLL that simply calls the needed registry API's to see if that would work without the need for an application with elevated privileges. Currently, the Java code saves and restores IE settings by running regedit.exe. Saving and restoring registry keys to a file using the registry API's requires enablement of SE_BACKUP_NAME and SE_RESTORE_NAME privileges for the process object, where the need to do this isn't as clear as it should be in the API documentation and exactly how to do this properly was not easy to find. Then, there were problems with RegSaveKeyEx in that what was desired was to use the REG_NO_COMPRESSION flag. Calling with this flag on Vista always results in an error, ERROR_INVALID_PARAMETER. It was desired to do this, so that the existing Java code to parse the saving registry information would not need any adjustments, it would just work. I found no comments or documentation anywhere that indicated this flag would not be valid on Vista or anywhere else. The text output may have been disabled on Vista to make it harder to programatically dump and read the registry contents, since one cannot easily parse and deal with the non-zip binary compressed format.
The default behavior of starting Visual Studio provides a prompt and recommends running Visual Studio with elevated privileges ("Run as Administrator"). The problem is that when running and debugging your application, the application inherits the Run as Administrator privileges. So, it turned out that RegSaveKeyEx and RegRestoreKey ONLY work with Administrator privileges even though the call to AdjustTokenPrivileges to enable SE_BACKUP_NAME and SE_RESTORE_NAME privileges on the process token succeeded. When calling RegSaveKeyEx or RegRestoreKey, the return is ERROR_PRIVILEGE_NOT_HELD unless running with elevated privilege. (Debugging and testing my code from an application was quicker and easier, to be moved to the DLL after it was working properly).
Ok, so now the solution: Calling the registry API's to read and to modify IE registry settings does work without elevated privilege (at least when running as a member of the administrator group, which is the normal use case). Code has been written to load a new DLL that will be installed in org.eclipse.hyades.test.core plugin directory. The DLL is statically linked to Mircosoft runtime libraries in order to avoid the need for added installation problems. All the registry calls will be made through JNI code in this DLL. The DLL will be loaded when a recording is started and will be unloaded when the workbench is closed. There are not very many registry keys involved, so completion of this work is now straight-forward to complete. On the plus side, one could argue we should be calling the native API's anyway as opposed to kicking off regedit to do our work.
Created attachment 76930 [details]
patch for org.eclipse.hyades.test.core
Adding patches for org.eclipse.hyades.test.core and org.eclipse.hyades.test.ui. In addition, will be adding a new TPTPRegistryUpdater.dll and source code for the C++ project into CVS.
If the DLL is not in the org.eclipse.hyades.test.core plugin directory at runtime, the behavior will be the same as it was before the defect fix.
Created attachment 76931 [details]
patch for org.eclipse.hyades.test.ui
The defect fix was reviewed with Joe Toomey. During the walk-through, it was found there was a random failure in the registry queries (RegQueryValueEx) that was only occurring with the release build of the DLL. Testing had been and remains satisfactory with the debug build of the DLL. Since there is not a fix in-hand that is deemed reliable, I am not able to deliver this fix before leaving on vacation. I am reluctant to defer this defect but I feel it is prudent to take the necessary time to fully test this implementation before packaging. Also, I would like for Duwayne to book some time (on the Test Project weekly call or a separate call) to do a walk-through of the implementation. We need to make this our #1 item once 4.4.1 is closed. Duwayne, please update the hours worked. (In reply to comment #30) > I am reluctant to defer this defect but I feel it is prudent to take the > necessary time to fully test this implementation before packaging. Also, I > would like for Duwayne to book some time (on the Test Project weekly call or a > separate call) to do a walk-through of the implementation. We need to make > this our #1 item once 4.4.1 is closed. > > Duwayne, please update the hours worked. > Given the age ans severity of the base nature of this defect. Is it acceptable to deffer until next year? Vista support is becoming a basic requirement and this is a pretty serious usability problem. Perhaps apply the reviewed fix now and further refine the solution in 4.5. (In reply to comment #31) > (In reply to comment #30) > > I am reluctant to defer this defect but I feel it is prudent to take the > > necessary time to fully test this implementation before packaging. Also, I > > would like for Duwayne to book some time (on the Test Project weekly call or a > > separate call) to do a walk-through of the implementation. We need to make > > this our #1 item once 4.4.1 is closed. > > > > Duwayne, please update the hours worked. > > > > Given the age ans severity of the base nature of this defect. Is it acceptable > to deffer until next year? Vista support is becoming a basic requirement and > this is a pretty serious usability problem. > > Perhaps apply the reviewed fix now and further refine the solution in 4.5. > Unfortunately, Joe and Duwayne are on vacation until next week, resulting in the earliest possible fix too late for 4.4.1 (~Sept. 5). There are two readme entries in place with easy work-arounds (http://www.eclipse.org/tptp/home/downloads/releasenotes/releasenotes4_4_0.html#4_CTI_10). (In reply to comment #32) I confirmed that no consuming products or the user community are requesting this defect in 4.4.1. The issue found that was causing a problem in comment 29. The problem was an uninitialized input/output parameter to RegQueryValueEx that specifies the size of the buffer passed in. I also added more error checking code. There is a new issue. It turned out that while FireFox works fine, Internet Explorer is evidently a protected process on Vista. One of the methods we use to stop the HTTP recording is to close the browser. The current code calls System.getRuntime().exec to start the browser application. We subsequently call Process.waitFor(). When the browser closes, the wait exits and we stop the recording. If we right click "Run as Administrator", this works fine. If we do not select "Run as Administrator", Process.waitFor() immediately returns, which then currently means we stop the recording. There is documentation around that mentions the potential problem: Protected Processes Windows Vista introduces protected processes to enhance support for Digital Rights Management. The system restricts access to protected processes and the threads of protected processes. The following standard access rights are not allowed from a process to a protected process: DELETE READ_CONTROL WRITE_DAC WRITE_OWNER The following specific access rights are not allowed from a process to a protected process: PROCESS_ALL_ACCESS PROCESS_CREATE_PROCESS PROCESS_CREATE_THREAD PROCESS_DUP_HANDLE PROCESS_QUERY_INFORMATION PROCESS_SET_INFORMATION PROCESS_SET_QUOTA PROCESS_VM_OPERATION PROCESS_VM_READ PROCESS_VM_WRITE The PROCESS_QUERY_LIMITED_INFORMATION right was introduced to provide access to a subset of the information available through PROCESS_QUERY_INFORMATION. I found no documentation listing what processes are protected. It appears that Internet Explorer is one of them. This would make sense, since that prevents installing spy hooks like we use in the Rational Robot VU recorder by blocking read and write permissions in the process VM unless "Run as Administrator" is selected. One might question why I did not use ShellExecuteEx. After reading the documentation, it appears that this API does not necessarily return a process handle. Further, it was stated that with IE, if the command line is to open a document, it may actually use an existing instance of IE through a DDE conversation. In any case, that did not sound reliable. An investigation was done to use the Win32 Process Status API's to see if that would work to implement the following approach in Native Code: 1. Get a list of all process ID's running on the system using EnumProcesses. 2. Get a handle to each process using OpenProcess with permissions to SYNCHRONIZE and with PROCESS_QUERY_LIMITED_INFORMATION permissions. Note that not all processes allow this access. SYNCHRONIZE permission allows access to wait for the process object to terminate. 3. Find all currently running PID's for Internet Explorer. 4. Start Internet Explorer and repeat the above steps to find the instance of IE that was started. 5. Use WaitForSingleObject on Internet Explorer to initiate stopping the recording when IE is closed. 6. Modify the existing Java code to smoothly use the native code to accomplish the above steps. I have had success in getting through step 3. The rest would seem to be very straight forward to finish up on. As far as the Psapi.dll, it includes an open license to re-distribute (since it is a part of the Windows SDK). My plan was to statically link to the .lib file to avoid a complicated install change. I would imagine there will be some formal approval process to include this DLL for distribution in the TPTP install. Please feel free to throw out questions or ask for clarification. Just to clarify comment #34, the problem with IE was also checked in native code, using CreateProcess. The process ID returned by CreateProcess was NOT the process ID of IE. I suspect that a ShellExecute was done under the covers and the returned process information was for the shell, not IE. How convenient! Hence, this was not a bug in the JVM. Code to implement the strategy in comment 34 was written and tested. There was a race condition. When inspecting the list of processes immediately after calling CreateProcess on IE, we sometimes get the PID of the temporary IE process that was returned by the actual call to CreateProcess, not the PID of the IE that ends up running. There were comments I found somewhere that indicated IE immediately kicks off ieuser.exe and then gets executed again. I was considering a strategy to repeatedly get the list again with a small delay until we get the right instance of IE. Then, I ran across this: Launching and Navigating a Protected Mode Process If your application uses CreateProcess to launch IE, it should call IELaunchURL on Vista. This will ensure that your application gets the right return values and that IE launches in Protected mode for URLs whose zone has Protected mode on. If you need to determine whether a specific URL will open in a low (Protected mode) or a medium integrity IE process before launching IE, call IEIsProtectedModeURL. Note that a high integrity process with administrator privileges will launch a high integrity IE process with Protected mode off. If you want to launch Protected mode from your high integrity process then first create a medium integrity process, which will launch your high integrity process and IE. Testing so far indicates that IELaunchURL (which I believe was created for Vista and only exists on Vista) will work fine and we get the correct process ID. I have also added a native function to call TerminateProcess on the returned process ID for the case where the user selects the stop button to stop the recording (as opposed to closing Internet Explorer to stop the recording). It would be much better if Microsoft would expend the effort to document or at least reference how various API's act differently in Vista. For example, the CreateProcess API documentation should mention or reference Protected Mode Internet Exlorer documentation or the IELaunchURL API. I am now in the final code cleanup and testing effort. I plan to also test on a machine wihout Visual Studio to verify that I do not have any unwanted dependencies buried in the DLL. At least this new solution eliminates the need for the Process Status API's. There is an issue with IELaunchURL. The API fails to launch IE if the specified URL is "about:blank", which is what we normally use so that the user does not end up with a recording that includes his default page. Other URL's work fine. I tried various strings in a frustrating attempt to make this work to no avail and found no reference to this problem in searching the internet. So, the solution is to put an empty page in the plugin and launch IE to bring up that page. The page only contains <HTML></HTML>, the same contents as the about:blank page. After testing on another Vista machine, a minor change was needed for the build settings on the DLL that was incorrect. There was still a dependency on a Microsoft runtime DLL that needed to be eliminated. That problem is fixed. Finally, I thought there was a simple problem with the security settings on my Vista box. I was being prompted by UAC once per recording when not "Run as Admin" and simply hit cancel to force my code fix to execute. When running on another Vista box, I was getting the same result. (Prior to this back when this defect was filed we had seen a completely silent failure). So, the answer to this issue is to determine if we are running Vista, then make the native calls to open the process token for the current process, then call GetTokenInformation on TOKEN_ELEVATION_TYPE. If running elevated, handle the registry calls the way it was done previously. If not running elevated, use native code to do all the work. This has been tested, so now I am again in final cleanup and testing to have this fix reviewed. As of the discussion this morning, the intent is to request approval to deliver this fix into the 4.4.0.3 branch for the fall release. Scheduling a review meeting now. Correction to comment #37. The defect fix is to be applied to TPTP 4.4.1, now slated for a Winter release date. ******* SUMMARY OF DEFECT FIX ********** In view of all the history of this defect and the things that were tried and did not work, it is appropriate to now summarize the defect fix in what should remain as the final form: There is a new DLL that will be pre-built and checked into CVS under org.eclipse.hyades.test.core. The DLL has multiple purposes, which will be described. The DLL will only get loaded when performing an HTTP Recording on Windows Vista. The name of the DLL is open for further discussion. The changes apply only to recording with Internet Explorer. When the user starts the recording, Java code in org.eclipse.hyades.test.ui will check to see if the OS is Vista. If it is not Vista, all of the code to get and set registry settings and execute Internet Explorer will work the same as it did before this change. If the OS is Vista, the DLL will be loaded by a new JNIWrapper.java class in org.eclipse.hyades.test.core. A native function in the DLL will be called to find out if the Eclipse process is running with elevated privileges or not. If the process is running with elevated privileges, the behavior will revert to working the same as it did previously (same as Windows XP behavior). If the Eclipse process is not running with elevated privileges, the DLL will be used to retrieve needed registry settings, make changes to the registry setttings, execute Internet Explorer, wait for Internet Explorer to terminate or terminate the IE process if requested due to the user stopping the recording, and restoring the original IE registry settings. Registry values are obtained by making API calls to RegOpenKeyEx and RegQueryValueEx. These values are temporarily saved in a file under the workspace. Certain registry values are then changed using RegSetValueEx under HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Internet Settings to point IE to point to a proxy server, which is our proxy recorder. These values were also saved in a temporary file under the workspace. IE is launched by the DLL using a new API created for Vista, IELaunchURL. CreateProcess should not be used for this purpose, since the process ID returned is generally not the correct one (There is a more complex launching sequence for Protected Mode IE). From Java, we then call another function in the DLL to wait for IE to exit using OpenProcess with SYNCHRONIZE and PROCESS_TERMINATE permissions on the process ID returned from IELaunchURL, and then WaitForSingleObject. Alternatively, the user may manually stop the recording instead of closing IE. In this case, we call another function in the DLL, which immediately calls TerminateProcess on the process handle. IE then terminates, which also releases the thread above that is blocked by WaitForSingleObject. Now that the recording is terminated, we read the original registry values and restore the settings by calling the DLL again to execute RegSetValueEx. This completes the process and we are ready for the next recording. A couple of notes: Note that normally we launch IE with "about:blank". With the IELaunchURL, this fails. I found no solution that would work. So, the fix will include a blank.htm page that contains only the opening and closing tag (<HTML></HTML>. Also fixed was an IOException that happens only on Vista due to the fact that IE 7 on Vista only has not registry setting for JavaConsoleEnable. (In reply to comment #38) A conference call was held today with Joe, Mark, Duwayne and myself to code review the implementation. A few comments/questions remain from that call: Questions: -Has this fix been tested for multiple clients (e.g. two Eclipse workbenches recording at the same time) including starting, terminating, and completing a unique recording? -Has this fix been tested with IE 6 on XP and Vista? -How will zipping the org.eclipse.hyades.test.core plug-in affect loading the blank.html and DLL? -Where will the source code for the DLL be stored in CVS? -What will be the final name for the DLL? I have no preferences as long as it is descriptive and generic (e.g. TPTPRegistryUpdater.dll or similar). -What test cases have been added to the test bucket to exercise each code path (start , stop, and finish, recording with one/two clients with IE 6 and IE 7) on Vista, XP, and Linux? -Are all info/error messages externalized? Do all error messages have IDs? Have all debug messages (e.g. std out and err) removed? -Why do you write the new registry settings to a local file? Comments: -Store the files containing the old registry settings to the user's temporary directory (e.g. System.getProeprty("java.io.tmpdir") or File.createTempFile()). -Consider creating the blank.html at run-time and writing it to the user's temporary directory (e.g. System.getProeprty("java.io.tmpdir") or File.createTempFile()). This would eliminate any problems if this plug-in is zipped. If not, create a folder for the file instead of the plug-in's root directory. -The checkOSVersion() method on the Java side needs to be modified to do the following: if Vista{ if DLL found { if(run as administrator){ use original code path; } else{ use DLL; } } else{ dialog with work-around (e.g. run as administrator); } } else{ use original code path; } -Use the standard Java notation for naming methods (e.g. first letter is lowercase and all proper nouns are title case). -Change the <description> of the manifest to something more descriptive than 'who cares'. -The copyright comment needs to be added to all new files (see http://www.eclipse.org/tptp/home/documents/process/development/copyright.html). -The copyright dates needs to be updated for all existing files. -Please update the hours worked for this defect (as accurate as possible). -Please attach the complete patch to the Bugzilla. -Submit a defect questionnaire to the Test Project mailing list for project/PMC approval (see http://www.eclipse.org/tptp/home/documents/process/development/stop_ship_template.html). -Once this defect is fixed, the two readme entries can be removed in 4.5 since the 4.4.x readme is still required for 4.4.0. -Ensure all new APIs are marked internal or provisional (see http://www.eclipse.org/tptp/home/documents/process/development/api_contract.html). -We should consider using the same solution for IE on XP and Vista. Please open a defect to merge both code paths. Thanks Duwayne for all the effort (triage, investigation, design, implementation, debugging, testing, etc.) you have made on this defect. Candidate for 4.4.1. I am adding further comments to document recent changes that were needed pertaining to the protected mode operation of Internet Explorer. I have completed testing and I am satisfied that this fix is ready for delivery barring any futher comments. It turns out that IELaunchURL will fail when using NULL as the URL if the user has set "about:blank" as the default page, which is what we wanted. If the user has chosen a default URL to go to that is not "about:blank", the call succeeds. Conversely, if we pass in a URL to a local file to present a blank page, IELaunchURL is also fine. Thus, there is no sound and reliable direct approach for launching IE with the registry set to bring up about:blank. Previously, it was thought that the only problem was if "about:blank" was the URL we passed in to IELaunchURL, so I attempted a fix that would set the default page in the registry to "about:blank" only to find that this does not work either. This forced the circle back to the solution of bringing up a blank page from the local file system, which had already been tested as workable. The issue is that when IE then gets launched, it is a Medium Integrity process because it is bringing up a local file in the "Trusted Zone". As soon as we then try to navigate to an internet site, the IE broker process will launch another instance of IE as a Low Integrity process. When this instance comes up, you will see in the lower right that it is running with "Protected Mode On". There are now two instances of IE running. One thing to note that is even worse. If there is already a Low Integrity instance of IE running, the URL navigation pops up in a new tab on the running instance of IE, presummably through a DDE conversation. Also note that this does not happen if the user navigates to a web site that he has explicitly added to the list of trusted web sites. Note that when we were running eclipse with "Run as Administrator", none of this happened. We were running in a backward compatible mode kicking off IE as a High Integrity process with protected mode off, defeating the added Vista security. As a result of all this behavior, I have implemented the following solution: 1. Find any running instances of IE 2. If there are any running instances of IE using the process API's from the SDK, present an error message to close them first, like we do with FireFox (this avoids the user confusion of having navigation to a URL result in opening a tab in some running instance of IE and avoid the situation that we would not know when the user is finished recording). 3. Launch IE using IELaunchURL with a blank page in the temp directory 4. Look for the first traffic is detected by the Data Processor that contains a "Client Connect" message. When this happens, find the new instance of IE in DLL code, spin off a new thread, and wait for it to terminate as the end of the recording. If a new instance of IE never happens, stop the recording as usual when the first instance of IE closes (must have been to a trusted site). 5. If the recording is stopped via the stop button, terminate both instances of IE, as applicable The following are my responses to comment #39. Questions: -Has this fix been tested for multiple clients (e.g. two Eclipse workbenches recording at the same time) including starting, terminating, and completing a unique recording? This use case is not supported. Only one workbench can record IE at a time. If one attempts to record from a second workbench, the error message will indicate that IE is set to use a socks proxy. Recording from two workbenches at the same time would be very confusing. I believe one could record IE on one workbench while recording FireFox on another. However, this was not ever a use case that was considered to my knowledge and is not part of the existing design. -Has this fix been tested with IE 6 on XP and Vista? I have verified that the recorder continues to work properly on IE 6 with XP. Vista ships with IE 7 and IE 6 is not supported there. -How will zipping the org.eclipse.hyades.test.core plug-in affect loading the blank.html and DLL? The plugin would be broken. I assume others have tackled this issue and would plan to use the same approach to handle the DLL. I have implemented your suggestion of creating the blank.html file on the fly to avoid any issues with that file. -Where will the source code for the DLL be stored in CVS? In a separate folder under org.eclipse.hyades.test.core. -What will be the final name for the DLL? I have no preferences as long as it is descriptive and generic (e.g. TPTPRegistryUpdater.dll or similar). Due to the fact that users may install Eclipse under a lengthy directory tree, I have chosen TPTPRecUtil.dll. A longer name may cause issues with the limit of 260 characters on length of the path. -What test cases have been added to the test bucket to exercise each code path (start , stop, and finish, recording with one/two clients with IE 6 and IE 7) on Vista, XP, and Linux? There are no added test cases. We can now perform the test passes on Vista without starting Eclipse using "Run as Administrator". The other code path gets excercized when recording on XP. -Are all info/error messages externalized? Do all error messages have IDs? Yes. Have all debug messages (e.g. std out and err) removed? Yes. -Why do you write the new registry settings to a local file? I no longer do this. Comments: -Store the files containing the old registry settings to the user's temporary directory (e.g. System.getProeprty("java.io.tmpdir") or File.createTempFile()). I thought when we discussed this, we decided it was better to leave it in the workspace so that if there is a problem, the user could see the original settings. Especially since we delete the file when finished. In fact, this file might serve as the ideal way to recover from closing the workbench or other similar problems, as described in bugzilla 190476. -Consider creating the blank.html at run-time and writing it to the user's temporary directory (e.g. System.getProeprty("java.io.tmpdir") or File.createTempFile()). This would eliminate any problems if this plug-in is zipped. If not, create a folder for the file instead of the plug-in's root directory. I have implemented the temporary file solution and delete the file when the recording is completed. -The checkOSVersion() method on the Java side needs to be modified to do the following: if Vista{ if DLL found { if(run as administrator){ use original code path; } else{ use DLL; } } else{ dialog with work-around (e.g. run as administrator); } } else{ use original code path; } After thinking this comment over, my response is: In general, we do not put effort in to overcome an install issue. If we can't load the dll and call it, there is something wrong in the install. I think we should just print the exception message from Java like we do anywhere else when the class loader fails and leave it at that. So, this is the approach I have implemented. Created attachment 80259 [details]
TPTPRecUtil.dll
This attached DLL is to be installed in the plugin directory for org.eclipse.hyades.test.core.
Created attachment 80260 [details]
Patch for org.eclipse.hyades.test.core
Created attachment 80261 [details]
Patch for org.eclipse.hyades.test.ui
Created attachment 80262 [details]
Patch for org.eclipse.hyades.tools.ui
Updated patches for this defect fix. In org.eclipse.hyades.execution.recorder.Recorder.java, I needed to add a public method as follows:
/**
*
* returns the active recorder client application adapter
*/
public RecorderClient getRecorderClient(){
return client;
}
I needed this to access the InternetExlorerAdapter from the DataProcessor. Technically, this is adding public API, but it is really a very minor API that perhaps other application adapter writers may find useful. I would hope that we could add this very minor change without too much churn.
Created attachment 80270 [details]
This C++ file contains the DLL source code for this defect fix
Attached a copy of the C++ file that contains all the executable code used in the TPTPRecUtil.dll for this defect fix for independent review, as deemed appropriate. The Visual C++ project code, including this file, will be delivered into CVS following approval.
(In reply to comment #46) > Technically, this is adding public API, but it is really a very minor API that > perhaps other application adapter writers may find useful. I would hope that > we could add this very minor change without too much churn. > This seems like a valid addition, but minimally, you need to mark this method as javadoc provisional. (At which point it is provisional API, not yet public.) (In reply to comment #48) > (In reply to comment #46) > > > Technically, this is adding public API, but it is really a very minor API that > > perhaps other application adapter writers may find useful. I would hope that > > we could add this very minor change without too much churn. > > > > This seems like a valid addition, but minimally, you need to mark this method > as javadoc provisional. (At which point it is provisional API, not yet > public.) > Duwayne, please use the @provisional tag, as in the TPTP API Contract: http://www.eclipse.org/tptp/home/documents/process/development/api_contract.html (In reply to comment #41) A couple final comments: > 4. Look for the first traffic is detected by the Data Processor that contains > a "Client Connect" message. When this happens, find the new instance of IE in > DLL code, spin off a new thread, and wait for it to terminate as the end of the > recording. We need a dedicated test case to evaluate this every release in the event that the "Client Connect" message changes. > -Has this fix been tested for multiple clients (e.g. two Eclipse workbenches > recording at the same time) including starting, terminating, and completing a > unique recording? > > This use case is not supported. Only one workbench can record IE at a time. > If one attempts to record from a second workbench, the error message will > indicate that IE is set to use a socks proxy. Recording from two workbenches > at the same time would be very confusing. I believe one could record IE on one > workbench while recording FireFox on another. > > However, this was not ever a use case that was considered to my knowledge and > is not part of the existing design. Please open a Test.Doc defect to ensure this is clarified in the product documentation. > -Store the files containing the old registry settings to the user's temporary > directory (e.g. System.getProeprty("java.io.tmpdir") or File.createTempFile()). > > I thought when we discussed this, we decided it was better to leave it in the > workspace so that if there is a problem, the user could see the original > settings. Especially since we delete the file when finished. In fact, this > file might serve as the ideal way to recover from closing the workbench or > other similar problems, as described in bugzilla 190476. Your point is valid. > -The checkOSVersion() method on the Java side needs to be modified to do the > following: > > if Vista{ > if DLL found { > if(run as administrator){ > use original code path; > } > else{ > use DLL; > } > } > else{ > dialog with work-around (e.g. run as administrator); > } > } > else{ > use original code path; > } The intent of this comment was two-fold: 1) Logic for delegating to the DLL. 2) Error handling. For #2, we should be reporting the error as you say (corrupt install) but we should also suggest the work-around as well. Joe, please review before the Test Project call so we can discuss any questions you may have. Thanks. Created attachment 80900 [details]
Patch for org.eclipse.hyades.test.core
Created attachment 80901 [details]
Patch for org.eclipse.hyades.test.ui
Created attachment 80902 [details]
Patch for org.eclipse.hyades.tools.ui
Implemented requested changes in comment #48, #49, and #50. Also performed testing of the above patches on both Vista and Windows XP. Reviewed ad nauseum and approved. ;-) Defect fix was approved by PMC. Added a src-native directory and a zip version of the C++ project for building the TPTPRecUtil.dll, since this component will not be built during the normal build process. The pre-built TPTPRecUtil.dll was checked into CVS directly under the plugin directory. Delivered into both 4.4.1 and 4.5 branches. Updated hours worked on this defect. Fixed and verified, closing. |