Community
Participate
Working Groups
Create a Telnet connection. When connecting, enter an invalid user id or password, or specify invalid login/password/prompt in the connector service properties. --> The telnet connection tries connecting without success, no useful error message is given when it finally fails. --> When right-click > Launch shell was done, even the entire Eclipse IDE is blocked (see also bug #186569). I think that the following enhancements are needed: 1. During the login sequence in TelnetConnectorService, all prompts from the remote side should be recorded. If connect is unsuccessful for any reason, a dialog should be opened ("Connect unsuccessful. Remote side said:") in order to allow users fix the issue. The text for the dialog can be handled via a SystemMessageException, I think. 2. If, after sending a login to the remote side, the "login: " prompt is seen again, it was apparently an invalid login. Connect attempt should be canceled and the error shown as in (1). 3. Same in case sending a password was not sucessful. 4. The entire login sequence should have a user-configurable timeout (via the PropertySet). If the expected pattern for login/password/prompt is not received until the timeout, connect should be canceled and the error message (1) should be shown. 5. The default PROMPT expected should be more fuzzy. Tyipcal systems prompt with any character like #>%$ -- all of these should be allowed. In the PropertySet, either the special value AUTO should be used for this fallback, or a regular expression should be allowed to be able and pass multiple possible prompts. 6. (Optional) The TelnetConnectorService should be able to connect if the prompt is received even if login: and password: were not queried. This would allow for more successful logins even if the user doesn't configure the PropertySet properly. Especially in case the PROMPT is set to AUTO, any from the prompt characters can be matched at any time even before login: or password: were matched. I think that this is a major problem since it's just to easy to fall into the problem and not get any useful error message. It should be fixed ASAP.
Created attachment 67256 [details] patch for bug 186570 The contains the fix to handle incorrect logins more gracefully. After applying the patch, copy the icons folder from SSH plugin, because the telnet plugin is using a copy of the UserValidationDialog for prompting the user to enter login information if it fails. This dialog uses images form the icons folder, which may cause null pointer exceptions if it does not exist. When the login to telnet session times out due to multiple login attempts a SystemMessageException is thrown which currently does not pop up a dialog. The exception is written to console. Need to have a look at this
Created attachment 67384 [details] Updated patch against TM HEAD, Formatting changed only
Created attachment 67386 [details] Updated patch without UserValidationDialog Thanks for the patch, I reviewed it throughly. I do not like the idea of a UserValidationDialog for the following reasons: * A password entered in the UserValidationDialog will not be saved, so it is not available the next time. It's better to inform the user that login failed, so he can just try again and enter correct credentials in RSE's dialog rather than a special local dialog. In general, Credentials should be set and managed by the RSE framework only (if possible). Note that SSH is slightly different here, because it has the concept of a Passphrase and the concept of Keyboard-Interactive authentication. These work with a challenge-response kind of model, where it is not possible to know in advance what kind of information is needed, so in this special case it is necessary to do a special dialog other than RSE. But for telnet, I'd like to keep the RSE dialog only. I also slightly optimized the readUntil() method to not do indexOf() so often; and changed the String status of your connectThread to the int status you also use for readUntil(). These changes make the patch a lot simpler (95 lines of code) which I can apply without another IP Review. I'm applying the modified patch as attached.
On taking a second look, I also removed the "Telnet Only" systemType from your patch, since it is already defined in org.eclipse.rse.subsystems.shells.telnet This leaves the current patch with two modifications: - Connect done in a separate thread (that's good so it can be interrupted) - readUntil returns FAIL when pattern "closed" or "incorrect" is found. I also added the following additional modification - Use ProgressMonitor to check for cancel - Use a timeout for connect in case remote waits forever - Improve error reporting (see also bug #187218) But from the original bug report, still many issues are open. The items below reference the numbered items from the original description: 1. The text printed by the remote side during the login sequence is not yet logged or printed. For Reference, a Console could be used like FTP does, or the messages could be recorded as part of the SystemMessageException. Without knowing what the prompts from the remote side were, users for whom the login sequence fails have almost no chance getting the properties in the PropertySet right. 2., 3. are kind of addressed 4. Is addressed though the timeout is not configurable yet 5., 6. The default prompt is not fault tolerant enough yet. Many people will have different login prompts, so they will never get the "connected" status. At the very least, a regular expression like [#>%$] should be used to check for the prompt. Or, don't use a prompt by default at all and consider the connection connected already. Actually, this might be a solution for (1) as well -- show the entire login sequence in the shell being launched, and being very tolerant by allowing the "connected" status almost unconditionally. I'm keeping the bug open but moving severity down, since the connect now always happens on a background thread, and it is cancelable. This means that at the worst, the Telnet feature itself does not work, but the rest of Eclipse and RSE will be healthy. I'd still like to see this improved for 2.0 though. More patches will be appreciated, please contribute patches against CVS HEAD.
Closing this bug since the patch was applied. Remaining issues are tracked for post 2.0 in bug #194473.
[target cleanup] 2.0 RC1 was the original target milestone for this bug
Comment on attachment 67256 [details] patch for bug 186570 patch ended up being used
