| Summary: | RemoteCommandLauncher#execute for RDT will fail if any environment variable's value is null | ||
|---|---|---|---|
| Product: | [Tools] PTP | Reporter: | Corey Ashford <cjashfor> |
| Component: | RDT | Assignee: | 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
Corey Ashford
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.
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.
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 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. (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 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 '='.
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. 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. Created attachment 209601 [details]
Fix problems with environment variable parsing
Here's a new version, using git, and externalizing the error string.
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. 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. I need you to fill out a CLA before Git will let me commit this. Done just now, though I don't know why it's still showing me as -CLA. Takes some time to refresh... it's showing with a checkmark now. Still won't let me. "The author must register with Gerrit." May have to wait a bit longer: https://bugs.eclipse.org/bugs/show_bug.cgi?id=407114 Still the same message. Did you create an account for git.eclipse.org/r/ Committed to master. If you want this in 7.x as well let me know. |