Bugzilla – Full Text Bug Listing
| Summary: | RDTCommandLauncher#execute doesn't properly process environment variables | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Linux Tools | Reporter: | Corey Ashford <cjashfor> | ||||||||
| Component: | Project | Assignee: | Linux Distros Inbox <linux.distros-inbox> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | jjohnstn, obusatto | ||||||||
| Version: | unspecified | ||||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
(In reply to comment #0) > Created attachment 209951 [details] > Fix bugs in env var processing by using indexOf instead of split. > > The execute method in RDTCommandLauncher incorrectly deals with environment > variables in two cases: > > 1) If the value of the variable is null, the code will crash with an > index-out-of-range error due to indexing the tokens[] array (which is of length > 1) with the index [1]. > > 2) If the value of the env var contains one or more embedded '=' characters, > only the portion of the string leading up to the first '=' character is taken > as the value. The usage of the 2 in the split method should have handled case 2. If someone has: a=b=2 then token[0], "the name" is "a" and the value (token[1]) is then "b=2". The parameter 2 restricts the number of tokens created to an upper-maximum and the last token takes all characters to end of string, ignoring the tokenizer. The code could simply add a check on the length of the result array and then set the value to null in the empty case. I would assume that the internal code to do a String split is more efficient than us doing it ourselves manually. So, it could be: for (int i = 0; i < env.length; ++i) { String s = env[i]; String[] tokens = s.split("=", 2); if (tokens.length == 2) envMap.put(tokens[0], tokens[1]); else if (tokens.length == 1) envMap.put(tokens[0], null); else <raise exception or log error> } Good point about the "split("=", 2);" I had missed that detail and thought I had spotted a problem.
I will re-write this patch, using your technique. I think the "right thing" to do in this case is to log a warning. I think it won't come up often, but if it does we should let the user know, without breaking too many things.
Created attachment 210295 [details]
Fix two bugs in environment variable processing
I wrote this patch, and I have permission from my employer (IBM Corp.) to submit it.
What about the new corey's patch. Does it look fine? (In reply to comment #4) > What about the new corey's patch. Does it look fine? The new code needs a license/copyright notice that is EPL and Corey needs to add that his changes are EPL. Once that is done, I can check it in. Created attachment 215719 [details]
RDTCommandLauncher#execute: Fix problems with environment variables
License added. Waiting for Corey's statement. (In reply to comment #7) > License added. Waiting for Corey's statement. The patch I made is intended to be EPL. Comment on attachment 215719 [details]
RDTCommandLauncher#execute: Fix problems with environment variables
<250 lines so iplog+
Patch checked in. commit hash: 32911ffcb217e7b3ad4b91b35cca8c361bfa20a9 |
Created attachment 209951 [details] Fix bugs in env var processing by using indexOf instead of split. The execute method in RDTCommandLauncher incorrectly deals with environment variables in two cases: 1) If the value of the variable is null, the code will crash with an index-out-of-range error due to indexing the tokens[] array (which is of length 1) with the index [1]. 2) If the value of the env var contains one or more embedded '=' characters, only the portion of the string leading up to the first '=' character is taken as the value.