Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 368495

Summary: RemoteCommandLauncher#execute for RDT will fail if any environment variable's value is null
Product: [Tools] PTP Reporter: Corey Ashford <cjashfor>
Component: RDTAssignee: Chris Recoskie <recoskie>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: g.watson, ptp-inbox, recoskie, wainersm
Version: unspecified   
Target Milestone: 8.0   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Fix array out of range error by correctly dealing with the case where an env var has a null value
none
Fix array out of range error by correctly dealing with the case where an env var has a null value
none
Fix array out of range error by correctly dealing with the case where an env var has a null value
none
Fix problems with environment variable parsing none

Description Corey Ashford CLA 2012-01-12 19:09:02 EST
Build Identifier: M20110909-1335

If my environment has a variable with no value, in my case it is a variable called PDHOST, RemoteCommandLauncher#execute will fail with an array index out
of range error.

Reproducible: Always

Steps to Reproduce:
1.Before starting eclipse, do "export SOME_VAR="
2.Start up eclipse
3.Build a remotely-based project which relies on RDT.
4.Notice the array index out of range exception.
Comment 1 Corey Ashford CLA 2012-01-12 19:10:55 EST
Created attachment 209419 [details]
Fix array out of range error by correctly dealing with the case where an env var has a null value

Implemented by checking the result of the "split" method to see how many strings are returned, and act appropriately on that info.
Comment 2 Corey Ashford CLA 2012-01-12 19:15:06 EST
Created attachment 209420 [details]
Fix array out of range error by correctly dealing with the case where an env var has a null value

Removed debug line from previous version of the patch.
Comment 3 Chris Recoskie CLA 2012-01-13 09:45:52 EST
Seems straightforward enough.

For the cases where the number of strings returned is greater than two, it's hard to know what to do, as your patch indicates.  You don't know if the extra equal signs are to go into the variable name or the value, or both.  Not sure what to do here.  It's unfortunately a problem with the API we are forced to use.  Would it seem reasonable to you to assume that the variable name does not contain any equal signs?  I think that's the likely scenario.

Examples:

FOO=bar // nothing special

MAKE_COMMAND_LINE_OPTS=-k CFLAGS=-g /* this is entirely possible... assume everything before the first equal sign is the variable */

VARIABLE_WITH_EQUALS_=_IN_NAME=42 // this we won't handle correctly
Comment 4 Greg Watson CLA 2012-01-13 10:33:10 EST
If you go by shell rules, then 'a=b=c' results in variable 'a' containing 'b=c'. That seems a reasonable assumption. I think you can only create variables with an '=' in the name programmatically using setenv.
Comment 5 Corey Ashford CLA 2012-01-13 13:26:37 EST
(In reply to comment #4)
> If you go by shell rules, then 'a=b=c' results in variable 'a' containing
> 'b=c'. That seems a reasonable assumption. I think you can only create
> variables with an '=' in the name programmatically using setenv.

So the code really ought not use split at all, and instead just look for the first '=' sign, and create two substrings from the "prefix" and suffix of that equals.

- Corey
Comment 6 Corey Ashford CLA 2012-01-13 20:22:04 EST
Created attachment 209490 [details]
Fix array out of range error by correctly dealing with the case where an env var has a null value

Ok, how about this patch, which utilizes String#indexOf to find the first '=' and then divides the string on that position by using String#substring. If a variable has a null string value, place a null into the hash map.

I tested this and it appears to work correctly.  It also deals with environment variables whose value contain one or more '='.
Comment 7 Chris Recoskie CLA 2012-01-16 10:40:41 EST
Hmm... your patch does not apply properly under Git.  I think it's in CVS format?

Also, the error message should be properly externalized, as users can end up seeing it.
Comment 8 Corey Ashford CLA 2012-01-16 14:09:51 EST
Yeah, I made the patch using CVS.   I had put off switching over, but maybe now's the time.

I'll redo the patch, and post it a bit later today.
Comment 9 Corey Ashford CLA 2012-01-16 22:57:55 EST
Created attachment 209601 [details]
Fix problems with environment variable parsing

Here's a new version, using git, and externalizing the error string.
Comment 10 Corey Ashford CLA 2012-02-08 13:53:33 EST
Any idea when this patch (or whatever fix for this bug) will make it into a PTP release?

We are running into this bug fairly frequently, because IBM's Open Client Linux has an environment variable called "PDHOST" which is set to an empty string.
Comment 11 Wainer dos Santos Moschetta CLA 2012-03-14 09:38:46 EDT
any progress here?

As Corey mentioned it is an bug that affects us. I would like to get that patch applied as soon as possible if there isn't any technical impediment.
Comment 12 Chris Recoskie CLA 2013-10-28 12:53:16 EDT
I need you to fill out a CLA before Git will let me commit this.
Comment 13 Corey Ashford CLA 2013-10-28 13:07:29 EDT
Done just now, though I don't know why it's still showing me as -CLA.
Comment 14 Chris Recoskie CLA 2013-10-28 14:29:26 EDT
Takes some time to refresh... it's showing with a checkmark now.
Comment 15 Chris Recoskie CLA 2013-10-28 14:34:21 EDT
Still won't let me.

"The author must register with Gerrit."
Comment 16 Corey Ashford CLA 2013-10-28 16:33:50 EDT
May have to wait a bit longer: https://bugs.eclipse.org/bugs/show_bug.cgi?id=407114
Comment 17 Chris Recoskie CLA 2013-10-29 10:36:47 EDT
Still the same message.

Did you create an account for git.eclipse.org/r/
Comment 18 Chris Recoskie CLA 2013-10-29 14:16:50 EDT
Committed to master.

If you want this in 7.x as well let me know.