Bug 248900 - [launch] very long launch config name causes IOException and launch config will be removed
Summary: [launch] very long launch config name causes IOException and launch config wi...
Status: CLOSED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: bugday, helpwanted
Depends on:
Blocks:
 
Reported: 2008-09-29 04:02 EDT by Tobias Schwarz CLA
Modified: 2019-11-14 03:53 EST (History)
4 users (show)

See Also:


Attachments
Windows explorer message when copying directory with long file in it (25.50 KB, image/png)
2011-08-08 12:17 EDT, Helmut J. Haigermoser CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Schwarz CLA 2008-09-29 04:02:08 EDT
Build ID: 3.4

Steps To Reproduce:
1.create a java application launch config and name it "123"
2.reopen the launch configuration dialog and rename the launch config "123" to something very long (like 10 times the string "veryveryverylonglaunchconfigname")
3.click apply -> an IOException dialog pops up
4.close the exception dialog and the launch config dialog.
5.reopen the launch config dialog - the launch config was removed.


More information:
launch config name should be limmited by the entry field.
Comment 1 Curtis Windatt CLA 2008-09-29 10:27:48 EDT
The behaviour is handled differently by launch configuration types.  Java config throws up an error dialog, Plug-in tests trims the name and logs the error.  This is an OS limitation, but we should be warning the user, preferably before they hit apply.  Perhaps a warning or error should be put up.

Consider for 3.5.  Contribution welcome :)
Comment 2 Michael Rennie CLA 2008-10-15 10:09:02 EDT
Something to consider in the solution to this bug is that the name + the full OS path where it will be saved length must not exceed the OS limit, not just the name
Comment 3 Helmut J. Haigermoser CLA 2011-08-02 10:54:39 EDT
(In reply to comment #2)
> Something to consider in the solution to this bug is that the name + the full
> OS path where it will be saved length must not exceed the OS limit, not just
> the name
*ping*
Has there been any progress on this one? Testing with 3.7 the problem is still there, did we agree on a solution for this one? I see a few options:
- check file length against known OS limitations  (Windows, 256 chars I think?)
- name launch configs differently: launch<number>.launch, and have the name as property inside..

Helmut
Comment 4 Helmut J. Haigermoser CLA 2011-08-05 04:33:23 EDT
CQ:WIND00136203
Comment 5 Michael Rennie CLA 2011-08-08 09:44:10 EDT
(In reply to comment #3)

> Has there been any progress on this one? 

No, and contributions are still welcome :)

> did we agree on a solution for this one? I see a few options:
> - check file length against known OS limitations  (Windows, 256 chars I think?)

This would be the preferable way to do it, plus it is trivial to add in some extra validation during the save cycle

> - name launch configs differently: launch<number>.launch, and have the name as
> property inside..

This would break the current scheme used where configurations are referred to by their names - for example Ant project builders and shared launch configurations.
Comment 6 Helmut J. Haigermoser CLA 2011-08-08 10:24:52 EDT
(In reply to comment #5)
> (In reply to comment #3)
> 
> > Has there been any progress on this one? 
> 
> No, and contributions are still welcome :)
> 
> > did we agree on a solution for this one? I see a few options:
> > - check file length against known OS limitations  (Windows, 256 chars I think?)
> 
> This would be the preferable way to do it, plus it is trivial to add in some
> extra validation during the save cycle
> 

Since LaunchManager::isValidLaunchConfigurationName already contains such checks I tried the following:

if ((configname.length() + ".launch".length()) > 260) //$NON-NLS-1$
  throw new IllegalArgumentException("Error: > 260."); //$NON-NLS-1$


Unfortunately my Windows 7 host does not very much agree to the documented windows limit here, sometimes files with 255 char length throw an OS error, other times files with lengths greater 265 seem to be creatable. The full path has an influence there, so a bare check for file names will likely not be enough. Note that my Windows explorer seems to be in full agreement with the limit, it won't allow me to create files that are longer than 260 chars, and this includes the full path of the file as well. 

Going forward I suggest adding a check for 260 chars, but of the whole file name plus path. Unfortunately the LaunchManager does not know about the full File so this check would have to sit somewhere else, LaunchConfigurationTabGroupViewer::verifyName maybe? save methods might be an option, but those would not be run while the user is typing, so no direct feedback would be possible until the save failed.

Helmut
Comment 7 Pawel Piech CLA 2011-08-08 12:06:31 EDT
I'm kind of new to the problem, but it seems to me that there's a fundamental contradiction: if we limit the full path to 260, then what happens when the user copies the workspace deeper into the directory hierarchy?  Or is the problem taken care of by the fact that the windows explorer would prevent user from making such a copy?

If this is true, then could isValidLaunchConfigurationName() simply take into account the current workspace location?  This could be somewhat confusing to users, but so is windows in this area.
Comment 8 Helmut J. Haigermoser CLA 2011-08-08 12:17:07 EDT
Created attachment 201091 [details]
Windows explorer message when copying directory with long file in it
Comment 9 Helmut J. Haigermoser CLA 2011-08-08 12:20:01 EDT
(In reply to comment #7)
> I'm kind of new to the problem, but it seems to me that there's a fundamental
> contradiction: if we limit the full path to 260, then what happens when the
> user copies the workspace deeper into the directory hierarchy?  Or is the
> problem taken care of by the fact that the windows explorer would prevent user
> from making such a copy?
Yes, windows explorer will take care of this, see attached pic.
> 
> If this is true, then could isValidLaunchConfigurationName() simply take into
> account the current workspace location?  This could be somewhat confusing to
> users, but so is windows in this area.
Yes, the full path should be considered, maybe the signature of this method should change from (String name) to (Configuration configuration)?

Helmut
Comment 10 Helmut J. Haigermoser CLA 2011-08-08 12:21:23 EDT
> ...but so is windows in this area.
Not only in this ;)

But yes, this is very annoying. I've read some comments about "better" API calls that will allow much longer file names, but obviously java still uses the old ones...
Comment 11 Michael Rennie CLA 2011-08-08 12:33:26 EDT
(In reply to comment #9)
> Yes, the full path should be considered, maybe the signature of this method
> should change from (String name) to (Configuration configuration)?

I agree the full path must be considered, but we would have to add a new method like isValidLaunchConfigurationName(IContainer, String) (or similar) because the old method is API.

Also rather than adding new API we could just as easily make the fix local to LaunchConfigurationTabGroupViewer#verifyName.
Comment 12 Helmut J. Haigermoser CLA 2011-08-09 06:01:28 EDT
(In reply to comment #11)
> (In reply to comment #9)
> > Yes, the full path should be considered, maybe the signature of this method
> > should change from (String name) to (Configuration configuration)?
> 
> I agree the full path must be considered, but we would have to add a new method
> like isValidLaunchConfigurationName(IContainer, String) (or similar) because
> the old method is API.
> 
> Also rather than adding new API we could just as easily make the fix local to
> LaunchConfigurationTabGroupViewer#verifyName.

Agreed, something like this would do the trick in that method(first line exists today):
				mgr.isValidLaunchConfigurationName(currentName);
				IFile ifile = fWorkingCopy.getFile();
				File file = null;
				if (ifile == null && config.isLocal()){
					IPath path = config.getLocation();
					if (path != null)
						file = path.toFile();
				} else {
					URI uri = ifile.getRawLocationURI();
					if (uri != null)
						file = new File(uri);
				}
				if (file !=null && file.getAbsolutePath().length() > 260)
					throw new IllegalArgumentException("That name is too long for the windows operating system, please shorten it");
Comment 13 Lars Vogel CLA 2019-11-14 03:53:38 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

If the bug is still relevant, please remove the "stalebug" whiteboard tag.